From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from proxy.6wind.com (host.76.145.23.62.rev.coltfrance.com [62.23.145.76]) by dpdk.org (Postfix) with ESMTP id 79CC54C72 for ; Tue, 27 Feb 2018 16:11:45 +0100 (CET) Received: from glumotte.dev.6wind.com. (unknown [10.16.0.195]) by proxy.6wind.com (Postfix) with ESMTP id AFB2413AD44; Tue, 27 Feb 2018 16:08:16 +0100 (CET) From: Olivier Matz To: dev@dpdk.org Cc: Thomas Monjalon , Ferruh Yigit Date: Tue, 27 Feb 2018 16:11:29 +0100 Message-Id: <20180227151129.30387-1-olivier.matz@6wind.com> X-Mailer: git-send-email 2.11.0 Subject: [dpdk-dev] [PATCH] ethdev: return diagnostic when setting MAC address X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 27 Feb 2018 15:11:45 -0000 Change the prototype and the behavior of dev_ops->eth_mac_addr_set(): a return code is added to notify the caller (librte_ether) if an error occurred in the PMD. The new default MAC address is now copied in dev->data->mac_addrs[0] only if the operation is successful. The patch also updates all the PMDs accordingly. Signed-off-by: Olivier Matz --- Hi, This patch is the following of the discussion we had in this thread: https://dpdk.org/dev/patchwork/patch/32284/ I did my best to keep the consistency inside the PMDs. The behavior of eth_mac_addr_set() is inspired from other fonctions in the same PMD, usually eth_mac_addr_add(). For instance: - dpaa and dpaa2 return 0 on error. - some PMDs (bnxt, mlx5, ...?) do not return a -errno code (-1 or positive values). - some PMDs (avf, tap) check if the address is the same and return 0 in that case. This could go in generic code? I tried to use the following errors when relevant: - -EPERM when a VF is not allowed to do a change - -ENOTSUP if the function is not supported - -EIO if this is an unknown error from lower layer (hw or sdk) - -EINVAL for other unknown errors Please, PMD maintainers, feel free to comment if you ahve specific needs for your driver. Thanks Olivier doc/guides/rel_notes/deprecation.rst | 8 -------- drivers/net/ark/ark_ethdev.c | 9 ++++++--- drivers/net/avf/avf_ethdev.c | 12 ++++++++---- drivers/net/bnxt/bnxt_ethdev.c | 10 ++++++---- drivers/net/bonding/rte_eth_bond_pmd.c | 8 ++++++-- drivers/net/dpaa/dpaa_ethdev.c | 4 +++- drivers/net/dpaa2/dpaa2_ethdev.c | 6 ++++-- drivers/net/e1000/igb_ethdev.c | 12 +++++++----- drivers/net/failsafe/failsafe_ops.c | 16 +++++++++++++--- drivers/net/i40e/i40e_ethdev.c | 24 ++++++++++++++--------- drivers/net/i40e/i40e_ethdev_vf.c | 12 +++++++----- drivers/net/ixgbe/ixgbe_ethdev.c | 13 ++++++++----- drivers/net/mlx4/mlx4.h | 2 +- drivers/net/mlx4/mlx4_ethdev.c | 7 +++++-- drivers/net/mlx5/mlx5.h | 2 +- drivers/net/mlx5/mlx5_mac.c | 7 +++++-- drivers/net/mrvl/mrvl_ethdev.c | 7 ++++++- drivers/net/null/rte_eth_null.c | 3 ++- drivers/net/octeontx/octeontx_ethdev.c | 4 +++- drivers/net/qede/qede_ethdev.c | 7 +++---- drivers/net/sfc/sfc_ethdev.c | 14 +++++--------- drivers/net/szedata2/rte_eth_szedata2.c | 3 ++- drivers/net/tap/rte_eth_tap.c | 34 +++++++++++++++++++++------------ drivers/net/virtio/virtio_ethdev.c | 15 ++++++++++----- drivers/net/vmxnet3/vmxnet3_ethdev.c | 5 +++-- lib/librte_ether/rte_ethdev.c | 7 +++++-- lib/librte_ether/rte_ethdev_core.h | 2 +- test/test/virtual_pmd.c | 3 ++- 28 files changed, 159 insertions(+), 97 deletions(-) diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst index 74c18ed7c..2bf360f0d 100644 --- a/doc/guides/rel_notes/deprecation.rst +++ b/doc/guides/rel_notes/deprecation.rst @@ -134,14 +134,6 @@ Deprecation Notices between the VF representor and the VF or the parent PF. Those new fields are to be included in ``rte_eth_dev_info`` struct. -* ethdev: The prototype and the behavior of - ``dev_ops->eth_mac_addr_set()`` will change in v18.05. A return code - will be added to notify the caller if an error occurred in the PMD. In - ``rte_eth_dev_default_mac_addr_set()``, the new default MAC address - will be copied in ``dev->data->mac_addrs[0]`` only if the operation is - successful. This modification will only impact the PMDs, not the - applications. - * ethdev: functions add rx/tx callback will return named opaque type ``rte_eth_add_rx_callback()``, ``rte_eth_add_first_rx_callback()`` and ``rte_eth_add_tx_callback()`` functions currently return callback object as diff --git a/drivers/net/ark/ark_ethdev.c b/drivers/net/ark/ark_ethdev.c index ff87c20e2..3fc40cd74 100644 --- a/drivers/net/ark/ark_ethdev.c +++ b/drivers/net/ark/ark_ethdev.c @@ -69,7 +69,7 @@ static int eth_ark_dev_set_link_down(struct rte_eth_dev *dev); static int eth_ark_dev_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats); static void eth_ark_dev_stats_reset(struct rte_eth_dev *dev); -static void eth_ark_set_default_mac_addr(struct rte_eth_dev *dev, +static int eth_ark_set_default_mac_addr(struct rte_eth_dev *dev, struct ether_addr *mac_addr); static int eth_ark_macaddr_add(struct rte_eth_dev *dev, struct ether_addr *mac_addr, @@ -887,16 +887,19 @@ eth_ark_macaddr_remove(struct rte_eth_dev *dev, uint32_t index) ark->user_data[dev->data->port_id]); } -static void +static int eth_ark_set_default_mac_addr(struct rte_eth_dev *dev, struct ether_addr *mac_addr) { struct ark_adapter *ark = (struct ark_adapter *)dev->data->dev_private; - if (ark->user_ext.mac_addr_set) + if (ark->user_ext.mac_addr_set) { ark->user_ext.mac_addr_set(dev, mac_addr, ark->user_data[dev->data->port_id]); + return 0; + } + return -ENOTSUP; } static int diff --git a/drivers/net/avf/avf_ethdev.c b/drivers/net/avf/avf_ethdev.c index 4df661705..0866f0bed 100644 --- a/drivers/net/avf/avf_ethdev.c +++ b/drivers/net/avf/avf_ethdev.c @@ -65,7 +65,7 @@ static int avf_dev_rss_hash_update(struct rte_eth_dev *dev, static int avf_dev_rss_hash_conf_get(struct rte_eth_dev *dev, struct rte_eth_rss_conf *rss_conf); static int avf_dev_mtu_set(struct rte_eth_dev *dev, uint16_t mtu); -static void avf_dev_set_default_mac_addr(struct rte_eth_dev *dev, +static int avf_dev_set_default_mac_addr(struct rte_eth_dev *dev, struct ether_addr *mac_addr); static int avf_dev_rx_queue_intr_enable(struct rte_eth_dev *dev, uint16_t queue_id); @@ -926,7 +926,7 @@ avf_dev_mtu_set(struct rte_eth_dev *dev, uint16_t mtu) return ret; } -static void +static int avf_dev_set_default_mac_addr(struct rte_eth_dev *dev, struct ether_addr *mac_addr) { @@ -940,11 +940,11 @@ avf_dev_set_default_mac_addr(struct rte_eth_dev *dev, perm_addr = (struct ether_addr *)hw->mac.perm_addr; if (is_same_ether_addr(mac_addr, old_addr)) - return; + return 0; /* If the MAC address is configured by host, skip the setting */ if (is_valid_assigned_ether_addr(perm_addr)) - return; + return -EPERM; ret = avf_add_del_eth_addr(adapter, old_addr, FALSE); if (ret) @@ -968,7 +968,11 @@ avf_dev_set_default_mac_addr(struct rte_eth_dev *dev, mac_addr->addr_bytes[4], mac_addr->addr_bytes[5]); + if (ret) + return -EIO; + ether_addr_copy(mac_addr, (struct ether_addr *)hw->mac.addr); + return 0; } static int diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c index 21c46f833..8eb923ebb 100644 --- a/drivers/net/bnxt/bnxt_ethdev.c +++ b/drivers/net/bnxt/bnxt_ethdev.c @@ -1398,7 +1398,7 @@ bnxt_vlan_offload_set_op(struct rte_eth_dev *dev, int mask) return 0; } -static void +static int bnxt_set_default_mac_addr_op(struct rte_eth_dev *dev, struct ether_addr *addr) { struct bnxt *bp = (struct bnxt *)dev->data->dev_private; @@ -1408,7 +1408,7 @@ bnxt_set_default_mac_addr_op(struct rte_eth_dev *dev, struct ether_addr *addr) int rc; if (BNXT_VF(bp)) - return; + return -EPERM; memcpy(bp->mac_addr, addr, sizeof(bp->mac_addr)); @@ -1418,7 +1418,7 @@ bnxt_set_default_mac_addr_op(struct rte_eth_dev *dev, struct ether_addr *addr) continue; rc = bnxt_hwrm_clear_l2_filter(bp, filter); if (rc) - break; + return rc; memcpy(filter->l2_addr, bp->mac_addr, ETHER_ADDR_LEN); memset(filter->l2_addr_mask, 0xff, ETHER_ADDR_LEN); filter->flags |= HWRM_CFA_L2_FILTER_ALLOC_INPUT_FLAGS_PATH_RX; @@ -1427,10 +1427,12 @@ bnxt_set_default_mac_addr_op(struct rte_eth_dev *dev, struct ether_addr *addr) HWRM_CFA_L2_FILTER_ALLOC_INPUT_ENABLES_L2_ADDR_MASK; rc = bnxt_hwrm_set_l2_filter(bp, vnic->fw_vnic_id, filter); if (rc) - break; + return rc; filter->mac_index = 0; PMD_DRV_LOG(DEBUG, "Set MAC addr\n"); } + + return 0; } static int diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c index c34c3251f..979d8efcd 100644 --- a/drivers/net/bonding/rte_eth_bond_pmd.c +++ b/drivers/net/bonding/rte_eth_bond_pmd.c @@ -2851,11 +2851,15 @@ bond_ethdev_mtu_set(struct rte_eth_dev *dev, uint16_t mtu) return 0; } -static void +static int bond_ethdev_mac_address_set(struct rte_eth_dev *dev, struct ether_addr *addr) { - if (mac_address_set(dev, addr)) + if (mac_address_set(dev, addr)) { RTE_BOND_LOG(ERR, "Failed to update MAC address"); + return -EINVAL; + } + + return 0; } const struct eth_dev_ops default_dev_ops = { diff --git a/drivers/net/dpaa/dpaa_ethdev.c b/drivers/net/dpaa/dpaa_ethdev.c index 9b69ef456..8227446e4 100644 --- a/drivers/net/dpaa/dpaa_ethdev.c +++ b/drivers/net/dpaa/dpaa_ethdev.c @@ -813,7 +813,7 @@ dpaa_dev_remove_mac_addr(struct rte_eth_dev *dev, fman_if_clear_mac_addr(dpaa_intf->fif, index); } -static void +static int dpaa_dev_set_mac_addr(struct rte_eth_dev *dev, struct ether_addr *addr) { @@ -825,6 +825,8 @@ dpaa_dev_set_mac_addr(struct rte_eth_dev *dev, ret = fman_if_add_mac_addr(dpaa_intf->fif, addr->addr_bytes, 0); if (ret) RTE_LOG(ERR, PMD, "error: Setting the MAC ADDR failed %d", ret); + + return 0; } static struct eth_dev_ops dpaa_devops = { diff --git a/drivers/net/dpaa2/dpaa2_ethdev.c b/drivers/net/dpaa2/dpaa2_ethdev.c index 09a11d65a..2d092e72c 100644 --- a/drivers/net/dpaa2/dpaa2_ethdev.c +++ b/drivers/net/dpaa2/dpaa2_ethdev.c @@ -1074,7 +1074,7 @@ dpaa2_dev_remove_mac_addr(struct rte_eth_dev *dev, "error: Removing the MAC ADDR failed: err = %d\n", ret); } -static void +static int dpaa2_dev_set_mac_addr(struct rte_eth_dev *dev, struct ether_addr *addr) { @@ -1086,7 +1086,7 @@ dpaa2_dev_set_mac_addr(struct rte_eth_dev *dev, if (dpni == NULL) { RTE_LOG(ERR, PMD, "dpni is NULL\n"); - return; + return -EINVAL; } ret = dpni_set_primary_mac_addr(dpni, CMD_PRI_LOW, @@ -1095,6 +1095,8 @@ dpaa2_dev_set_mac_addr(struct rte_eth_dev *dev, if (ret) RTE_LOG(ERR, PMD, "error: Setting the MAC ADDR failed %d\n", ret); + + return 0; } static int dpaa2_dev_stats_get(struct rte_eth_dev *dev, diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c index 3c5138dea..e5be0d4a8 100644 --- a/drivers/net/e1000/igb_ethdev.c +++ b/drivers/net/e1000/igb_ethdev.c @@ -146,7 +146,7 @@ static int eth_igb_rar_set(struct rte_eth_dev *dev, struct ether_addr *mac_addr, uint32_t index, uint32_t pool); static void eth_igb_rar_clear(struct rte_eth_dev *dev, uint32_t index); -static void eth_igb_default_mac_addr_set(struct rte_eth_dev *dev, +static int eth_igb_default_mac_addr_set(struct rte_eth_dev *dev, struct ether_addr *addr); static void igbvf_intr_disable(struct e1000_hw *hw); @@ -171,7 +171,7 @@ static int igbvf_vlan_filter_set(struct rte_eth_dev *dev, uint16_t vlan_id, int on); static int igbvf_set_vfta(struct e1000_hw *hw, uint16_t vid, bool on); static void igbvf_set_vfta_all(struct rte_eth_dev *dev, bool on); -static void igbvf_default_mac_addr_set(struct rte_eth_dev *dev, +static int igbvf_default_mac_addr_set(struct rte_eth_dev *dev, struct ether_addr *addr); static int igbvf_get_reg_length(struct rte_eth_dev *dev); static int igbvf_get_regs(struct rte_eth_dev *dev, @@ -3146,13 +3146,14 @@ eth_igb_rar_clear(struct rte_eth_dev *dev, uint32_t index) e1000_rar_set(hw, addr, index); } -static void +static int eth_igb_default_mac_addr_set(struct rte_eth_dev *dev, struct ether_addr *addr) { eth_igb_rar_clear(dev, 0); - eth_igb_rar_set(dev, (void *)addr, 0, 0); + + return 0; } /* * Virtual Function operations @@ -3504,7 +3505,7 @@ igbvf_vlan_filter_set(struct rte_eth_dev *dev, uint16_t vlan_id, int on) return 0; } -static void +static int igbvf_default_mac_addr_set(struct rte_eth_dev *dev, struct ether_addr *addr) { struct e1000_hw *hw = @@ -3512,6 +3513,7 @@ igbvf_default_mac_addr_set(struct rte_eth_dev *dev, struct ether_addr *addr) /* index is not used by rar_set() */ hw->mac.ops.rar_set(hw, (void *)addr, 0); + return 0; } diff --git a/drivers/net/failsafe/failsafe_ops.c b/drivers/net/failsafe/failsafe_ops.c index 057e435cf..d2f302ffb 100644 --- a/drivers/net/failsafe/failsafe_ops.c +++ b/drivers/net/failsafe/failsafe_ops.c @@ -997,16 +997,26 @@ fs_mac_addr_add(struct rte_eth_dev *dev, return 0; } -static void +static int fs_mac_addr_set(struct rte_eth_dev *dev, struct ether_addr *mac_addr) { struct sub_device *sdev; uint8_t i; + int ret; fs_lock(dev, 0); - FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) - rte_eth_dev_default_mac_addr_set(PORT_ID(sdev), mac_addr); + FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) { + ret = rte_eth_dev_default_mac_addr_set(PORT_ID(sdev), mac_addr); + if ((ret = fs_err(sdev, ret))) { + ERROR("Operation rte_eth_dev_mac_addr_set failed for sub_device %" + PRIu8 " with error %d", i, ret); + fs_unlock(dev, 0); + return ret; + } + } fs_unlock(dev, 0); + + return 0; } static int diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c index 508b4171c..455b1d0be 100644 --- a/drivers/net/i40e/i40e_ethdev.c +++ b/drivers/net/i40e/i40e_ethdev.c @@ -369,7 +369,7 @@ static int i40e_get_eeprom_length(struct rte_eth_dev *dev); static int i40e_get_eeprom(struct rte_eth_dev *dev, struct rte_dev_eeprom_info *eeprom); -static void i40e_set_default_mac_addr(struct rte_eth_dev *dev, +static int i40e_set_default_mac_addr(struct rte_eth_dev *dev, struct ether_addr *mac_addr); static int i40e_dev_mtu_set(struct rte_eth_dev *dev, uint16_t mtu); @@ -11249,8 +11249,8 @@ static int i40e_get_eeprom(struct rte_eth_dev *dev, return 0; } -static void i40e_set_default_mac_addr(struct rte_eth_dev *dev, - struct ether_addr *mac_addr) +static int i40e_set_default_mac_addr(struct rte_eth_dev *dev, + struct ether_addr *mac_addr) { struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private); struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private); @@ -11261,7 +11261,7 @@ static void i40e_set_default_mac_addr(struct rte_eth_dev *dev, if (!is_valid_assigned_ether_addr(mac_addr)) { PMD_DRV_LOG(ERR, "Tried to set invalid MAC address."); - return; + return -EINVAL; } TAILQ_FOREACH(f, &vsi->mac_list, next) { @@ -11271,25 +11271,31 @@ static void i40e_set_default_mac_addr(struct rte_eth_dev *dev, if (f == NULL) { PMD_DRV_LOG(ERR, "Failed to find filter for default mac"); - return; + return -EIO; } mac_filter = f->mac_info; ret = i40e_vsi_delete_mac(vsi, &mac_filter.mac_addr); if (ret != I40E_SUCCESS) { PMD_DRV_LOG(ERR, "Failed to delete mac filter"); - return; + return -EIO; } memcpy(&mac_filter.mac_addr, mac_addr, ETH_ADDR_LEN); ret = i40e_vsi_add_mac(vsi, &mac_filter); if (ret != I40E_SUCCESS) { PMD_DRV_LOG(ERR, "Failed to add mac filter"); - return; + return -EIO; } memcpy(&pf->dev_addr, mac_addr, ETH_ADDR_LEN); - i40e_aq_mac_address_write(hw, I40E_AQC_WRITE_TYPE_LAA_WOL, - mac_addr->addr_bytes, NULL); + ret = i40e_aq_mac_address_write(hw, I40E_AQC_WRITE_TYPE_LAA_WOL, + mac_addr->addr_bytes, NULL); + if (ret != I40E_SUCCESS) { + PMD_DRV_LOG(ERR, "Failed to change mac"); + return -EIO; + } + + return 0; } static int diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c index fd003fe01..df9709f80 100644 --- a/drivers/net/i40e/i40e_ethdev_vf.c +++ b/drivers/net/i40e/i40e_ethdev_vf.c @@ -120,7 +120,7 @@ static int i40evf_dev_rss_hash_update(struct rte_eth_dev *dev, static int i40evf_dev_rss_hash_conf_get(struct rte_eth_dev *dev, struct rte_eth_rss_conf *rss_conf); static int i40evf_dev_mtu_set(struct rte_eth_dev *dev, uint16_t mtu); -static void i40evf_set_default_mac_addr(struct rte_eth_dev *dev, +static int i40evf_set_default_mac_addr(struct rte_eth_dev *dev, struct ether_addr *mac_addr); static int i40evf_dev_rx_queue_intr_enable(struct rte_eth_dev *dev, uint16_t queue_id); @@ -2658,7 +2658,7 @@ i40evf_dev_mtu_set(struct rte_eth_dev *dev, uint16_t mtu) return ret; } -static void +static int i40evf_set_default_mac_addr(struct rte_eth_dev *dev, struct ether_addr *mac_addr) { @@ -2667,15 +2667,17 @@ i40evf_set_default_mac_addr(struct rte_eth_dev *dev, if (!is_valid_assigned_ether_addr(mac_addr)) { PMD_DRV_LOG(ERR, "Tried to set invalid MAC address."); - return; + return -EINVAL; } if (vf->flags & I40E_FLAG_VF_MAC_BY_PF) - return; + return -EPERM; i40evf_del_mac_addr_by_addr(dev, (struct ether_addr *)hw->mac.addr); - i40evf_add_mac_addr(dev, mac_addr, 0, 0); + if (i40evf_add_mac_addr(dev, mac_addr, 0, 0) != 0) + return -EIO; ether_addr_copy(mac_addr, (struct ether_addr *)hw->mac.addr); + return 0; } diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c index 448325857..4167b57e9 100644 --- a/drivers/net/ixgbe/ixgbe_ethdev.c +++ b/drivers/net/ixgbe/ixgbe_ethdev.c @@ -228,7 +228,7 @@ static void ixgbe_dev_interrupt_delayed_handler(void *param); static int ixgbe_add_rar(struct rte_eth_dev *dev, struct ether_addr *mac_addr, uint32_t index, uint32_t pool); static void ixgbe_remove_rar(struct rte_eth_dev *dev, uint32_t index); -static void ixgbe_set_default_mac_addr(struct rte_eth_dev *dev, +static int ixgbe_set_default_mac_addr(struct rte_eth_dev *dev, struct ether_addr *mac_addr); static void ixgbe_dcb_init(struct ixgbe_hw *hw, struct ixgbe_dcb_config *dcb_config); static bool is_device_supported(struct rte_eth_dev *dev, @@ -286,7 +286,7 @@ static int ixgbevf_add_mac_addr(struct rte_eth_dev *dev, struct ether_addr *mac_addr, uint32_t index, uint32_t pool); static void ixgbevf_remove_mac_addr(struct rte_eth_dev *dev, uint32_t index); -static void ixgbevf_set_default_mac_addr(struct rte_eth_dev *dev, +static int ixgbevf_set_default_mac_addr(struct rte_eth_dev *dev, struct ether_addr *mac_addr); static int ixgbe_syn_filter_get(struct rte_eth_dev *dev, struct rte_eth_syn_filter *filter); @@ -4853,14 +4853,15 @@ ixgbe_remove_rar(struct rte_eth_dev *dev, uint32_t index) ixgbe_clear_rar(hw, index); } -static void +static int ixgbe_set_default_mac_addr(struct rte_eth_dev *dev, struct ether_addr *addr) { struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev); ixgbe_remove_rar(dev, 0); - ixgbe_add_rar(dev, addr, 0, pci_dev->max_vfs); + + return 0; } static bool @@ -5983,12 +5984,14 @@ ixgbevf_remove_mac_addr(struct rte_eth_dev *dev, uint32_t index) } } -static void +static int ixgbevf_set_default_mac_addr(struct rte_eth_dev *dev, struct ether_addr *addr) { struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private); hw->mac.ops.set_rar(hw, 0, (void *)addr, 0, 0); + + return 0; } int diff --git a/drivers/net/mlx4/mlx4.h b/drivers/net/mlx4/mlx4.h index 19c8a223d..c107794ce 100644 --- a/drivers/net/mlx4/mlx4.h +++ b/drivers/net/mlx4/mlx4.h @@ -131,7 +131,7 @@ void mlx4_allmulticast_disable(struct rte_eth_dev *dev); void mlx4_mac_addr_remove(struct rte_eth_dev *dev, uint32_t index); int mlx4_mac_addr_add(struct rte_eth_dev *dev, struct ether_addr *mac_addr, uint32_t index, uint32_t vmdq); -void mlx4_mac_addr_set(struct rte_eth_dev *dev, struct ether_addr *mac_addr); +int mlx4_mac_addr_set(struct rte_eth_dev *dev, struct ether_addr *mac_addr); int mlx4_vlan_filter_set(struct rte_eth_dev *dev, uint16_t vlan_id, int on); int mlx4_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats); void mlx4_stats_reset(struct rte_eth_dev *dev); diff --git a/drivers/net/mlx4/mlx4_ethdev.c b/drivers/net/mlx4/mlx4_ethdev.c index 3bc692731..2442e16a6 100644 --- a/drivers/net/mlx4/mlx4_ethdev.c +++ b/drivers/net/mlx4/mlx4_ethdev.c @@ -701,11 +701,14 @@ mlx4_vlan_filter_set(struct rte_eth_dev *dev, uint16_t vlan_id, int on) * Pointer to Ethernet device structure. * @param mac_addr * MAC address to register. + * + * @return + * 0 on success, negative errno value otherwise and rte_errno is set. */ -void +int mlx4_mac_addr_set(struct rte_eth_dev *dev, struct ether_addr *mac_addr) { - mlx4_mac_addr_add(dev, mac_addr, 0, 0); + return mlx4_mac_addr_add(dev, mac_addr, 0, 0); } /** diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h index 965c19f21..42e58d7f7 100644 --- a/drivers/net/mlx5/mlx5.h +++ b/drivers/net/mlx5/mlx5.h @@ -241,7 +241,7 @@ int priv_get_mac(struct priv *, uint8_t (*)[ETHER_ADDR_LEN]); void mlx5_mac_addr_remove(struct rte_eth_dev *, uint32_t); int mlx5_mac_addr_add(struct rte_eth_dev *, struct ether_addr *, uint32_t, uint32_t); -void mlx5_mac_addr_set(struct rte_eth_dev *, struct ether_addr *); +int mlx5_mac_addr_set(struct rte_eth_dev *, struct ether_addr *); /* mlx5_rss.c */ diff --git a/drivers/net/mlx5/mlx5_mac.c b/drivers/net/mlx5/mlx5_mac.c index e8a8d4594..0dc4bec46 100644 --- a/drivers/net/mlx5/mlx5_mac.c +++ b/drivers/net/mlx5/mlx5_mac.c @@ -118,10 +118,13 @@ mlx5_mac_addr_add(struct rte_eth_dev *dev, struct ether_addr *mac, * Pointer to Ethernet device structure. * @param mac_addr * MAC address to register. + * + * @return + * 0 on success. */ -void +int mlx5_mac_addr_set(struct rte_eth_dev *dev, struct ether_addr *mac_addr) { DEBUG("%p: setting primary MAC address", (void *)dev); - mlx5_mac_addr_add(dev, mac_addr, 0, 0); + return mlx5_mac_addr_add(dev, mac_addr, 0, 0); } diff --git a/drivers/net/mrvl/mrvl_ethdev.c b/drivers/net/mrvl/mrvl_ethdev.c index 705c4bd8b..8e14e54e2 100644 --- a/drivers/net/mrvl/mrvl_ethdev.c +++ b/drivers/net/mrvl/mrvl_ethdev.c @@ -952,6 +952,9 @@ mrvl_mac_addr_add(struct rte_eth_dev *dev, struct ether_addr *mac_addr, * Pointer to Ethernet device structure. * @param mac_addr * MAC address to register. + * + * @return + * 0 on success, negative error value otherwise. */ static void mrvl_mac_addr_set(struct rte_eth_dev *dev, struct ether_addr *mac_addr) @@ -960,7 +963,7 @@ mrvl_mac_addr_set(struct rte_eth_dev *dev, struct ether_addr *mac_addr) int ret; if (!priv->ppio) - return; + return -EPERM; ret = pp2_ppio_set_mac_addr(priv->ppio, mac_addr->addr_bytes); if (ret) { @@ -968,6 +971,8 @@ mrvl_mac_addr_set(struct rte_eth_dev *dev, struct ether_addr *mac_addr) ether_format_addr(buf, sizeof(buf), mac_addr); RTE_LOG(ERR, PMD, "Failed to set mac to %s\n", buf); } + + return ret; } /** diff --git a/drivers/net/null/rte_eth_null.c b/drivers/net/null/rte_eth_null.c index d003b2839..3b659cd23 100644 --- a/drivers/net/null/rte_eth_null.c +++ b/drivers/net/null/rte_eth_null.c @@ -461,10 +461,11 @@ eth_rss_hash_conf_get(struct rte_eth_dev *dev, return 0; } -static void +static int eth_mac_address_set(__rte_unused struct rte_eth_dev *dev, __rte_unused struct ether_addr *addr) { + return 0; } static const struct eth_dev_ops ops = { diff --git a/drivers/net/octeontx/octeontx_ethdev.c b/drivers/net/octeontx/octeontx_ethdev.c index b739c0b39..fabf86aa5 100644 --- a/drivers/net/octeontx/octeontx_ethdev.c +++ b/drivers/net/octeontx/octeontx_ethdev.c @@ -594,7 +594,7 @@ octeontx_dev_stats_reset(struct rte_eth_dev *dev) octeontx_port_stats_clr(nic); } -static void +static int octeontx_dev_default_mac_addr_set(struct rte_eth_dev *dev, struct ether_addr *addr) { @@ -605,6 +605,8 @@ octeontx_dev_default_mac_addr_set(struct rte_eth_dev *dev, if (ret != 0) octeontx_log_err("failed to set MAC address on port %d", nic->port_id); + + return ret; } static void diff --git a/drivers/net/qede/qede_ethdev.c b/drivers/net/qede/qede_ethdev.c index a91f43683..e4cd89511 100644 --- a/drivers/net/qede/qede_ethdev.c +++ b/drivers/net/qede/qede_ethdev.c @@ -1001,7 +1001,7 @@ qede_mac_addr_remove(struct rte_eth_dev *eth_dev, uint32_t index) ecore_filter_ucast_cmd(edev, &ucast, ECORE_SPQ_MODE_CB, NULL); } -static void +static int qede_mac_addr_set(struct rte_eth_dev *eth_dev, struct ether_addr *mac_addr) { struct qede_dev *qdev = QEDE_INIT_QDEV(eth_dev); @@ -1010,12 +1010,11 @@ qede_mac_addr_set(struct rte_eth_dev *eth_dev, struct ether_addr *mac_addr) if (IS_VF(edev) && !ecore_vf_check_mac(ECORE_LEADING_HWFN(edev), mac_addr->addr_bytes)) { DP_ERR(edev, "Setting MAC address is not allowed\n"); - ether_addr_copy(&qdev->primary_mac, - ð_dev->data->mac_addrs[0]); - return; + return -EPERM; } qede_mac_addr_add(eth_dev, mac_addr, 0, 0); + return 0; } static void qede_config_accept_any_vlan(struct qede_dev *qdev, bool flg) diff --git a/drivers/net/sfc/sfc_ethdev.c b/drivers/net/sfc/sfc_ethdev.c index 89a452907..b76d4bb2c 100644 --- a/drivers/net/sfc/sfc_ethdev.c +++ b/drivers/net/sfc/sfc_ethdev.c @@ -920,7 +920,7 @@ sfc_dev_set_mtu(struct rte_eth_dev *dev, uint16_t mtu) SFC_ASSERT(rc > 0); return -rc; } -static void +static int sfc_mac_addr_set(struct rte_eth_dev *dev, struct ether_addr *mac_addr) { struct sfc_adapter *sa = dev->data->dev_private; @@ -939,13 +939,14 @@ sfc_mac_addr_set(struct rte_eth_dev *dev, struct ether_addr *mac_addr) if (port->isolated) { sfc_err(sa, "isolated mode is active on the port"); sfc_err(sa, "will not set MAC address"); + rc = ENOTSUP; goto unlock; } if (sa->state != SFC_ADAPTER_STARTED) { sfc_info(sa, "the port is not started"); sfc_info(sa, "the new MAC address will be set on port start"); - + rc = EINVAL; goto unlock; } @@ -982,14 +983,9 @@ sfc_mac_addr_set(struct rte_eth_dev *dev, struct ether_addr *mac_addr) } unlock: - /* - * In the case of failure sa->port->default_mac_addr does not - * need rollback since no error code is returned, and the upper - * API will anyway update the external MAC address storage. - * To be consistent with that new value it is better to keep - * the device private value the same. - */ sfc_adapter_unlock(sa); + SFC_ASSERT(rc >= 0); + return -rc; } diff --git a/drivers/net/szedata2/rte_eth_szedata2.c b/drivers/net/szedata2/rte_eth_szedata2.c index e53c738db..1a1e2356b 100644 --- a/drivers/net/szedata2/rte_eth_szedata2.c +++ b/drivers/net/szedata2/rte_eth_szedata2.c @@ -1315,10 +1315,11 @@ eth_tx_queue_setup(struct rte_eth_dev *dev, return 0; } -static void +static int eth_mac_addr_set(struct rte_eth_dev *dev __rte_unused, struct ether_addr *mac_addr __rte_unused) { + return 0; } static void diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c index f09db0ea9..ac99b233c 100644 --- a/drivers/net/tap/rte_eth_tap.c +++ b/drivers/net/tap/rte_eth_tap.c @@ -929,48 +929,58 @@ tap_allmulti_disable(struct rte_eth_dev *dev) tap_flow_implicit_destroy(pmd, TAP_REMOTE_ALLMULTI); } -static void +static int tap_mac_set(struct rte_eth_dev *dev, struct ether_addr *mac_addr) { struct pmd_internals *pmd = dev->data->dev_private; enum ioctl_mode mode = LOCAL_ONLY; struct ifreq ifr; + int ret; if (is_zero_ether_addr(mac_addr)) { RTE_LOG(ERR, PMD, "%s: can't set an empty MAC address\n", dev->device->name); - return; + return -EINVAL; } /* Check the actual current MAC address on the tap netdevice */ - if (tap_ioctl(pmd, SIOCGIFHWADDR, &ifr, 0, LOCAL_ONLY) < 0) - return; + ret = tap_ioctl(pmd, SIOCGIFHWADDR, &ifr, 0, LOCAL_ONLY); + if (ret < 0) + return ret; if (is_same_ether_addr((struct ether_addr *)&ifr.ifr_hwaddr.sa_data, mac_addr)) - return; + return 0; /* Check the current MAC address on the remote */ - if (tap_ioctl(pmd, SIOCGIFHWADDR, &ifr, 0, REMOTE_ONLY) < 0) - return; + ret = tap_ioctl(pmd, SIOCGIFHWADDR, &ifr, 0, REMOTE_ONLY); + if (ret < 0) + return ret; if (!is_same_ether_addr((struct ether_addr *)&ifr.ifr_hwaddr.sa_data, mac_addr)) mode = LOCAL_AND_REMOTE; ifr.ifr_hwaddr.sa_family = AF_LOCAL; rte_memcpy(ifr.ifr_hwaddr.sa_data, mac_addr, ETHER_ADDR_LEN); - if (tap_ioctl(pmd, SIOCSIFHWADDR, &ifr, 1, mode) < 0) - return; + ret = tap_ioctl(pmd, SIOCSIFHWADDR, &ifr, 1, mode); + if (ret < 0) + return ret; rte_memcpy(&pmd->eth_addr, mac_addr, ETHER_ADDR_LEN); if (pmd->remote_if_index && !pmd->flow_isolate) { /* Replace MAC redirection rule after a MAC change */ - if (tap_flow_implicit_destroy(pmd, TAP_REMOTE_LOCAL_MAC) < 0) { + ret = tap_flow_implicit_destroy(pmd, TAP_REMOTE_LOCAL_MAC); + if (ret < 0) { RTE_LOG(ERR, PMD, "%s: Couldn't delete MAC redirection rule\n", dev->device->name); - return; + return ret; } - if (tap_flow_implicit_create(pmd, TAP_REMOTE_LOCAL_MAC) < 0) + ret = tap_flow_implicit_create(pmd, TAP_REMOTE_LOCAL_MAC); + if (ret < 0) { RTE_LOG(ERR, PMD, "%s: Couldn't add MAC redirection rule\n", dev->device->name); + return ret; + } } + + return 0; } static int diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c index 884f74ad0..38a7a14f7 100644 --- a/drivers/net/virtio/virtio_ethdev.c +++ b/drivers/net/virtio/virtio_ethdev.c @@ -68,7 +68,7 @@ static int virtio_mac_addr_add(struct rte_eth_dev *dev, struct ether_addr *mac_addr, uint32_t index, uint32_t vmdq); static void virtio_mac_addr_remove(struct rte_eth_dev *dev, uint32_t index); -static void virtio_mac_addr_set(struct rte_eth_dev *dev, +static int virtio_mac_addr_set(struct rte_eth_dev *dev, struct ether_addr *mac_addr); static int virtio_intr_enable(struct rte_eth_dev *dev); @@ -1097,7 +1097,7 @@ virtio_mac_addr_remove(struct rte_eth_dev *dev, uint32_t index) virtio_mac_table_set(hw, uc, mc); } -static void +static int virtio_mac_addr_set(struct rte_eth_dev *dev, struct ether_addr *mac_addr) { struct virtio_hw *hw = dev->data->dev_private; @@ -1113,9 +1113,14 @@ virtio_mac_addr_set(struct rte_eth_dev *dev, struct ether_addr *mac_addr) ctrl.hdr.cmd = VIRTIO_NET_CTRL_MAC_ADDR_SET; memcpy(ctrl.data, mac_addr, ETHER_ADDR_LEN); - virtio_send_command(hw->cvq, &ctrl, &len, 1); - } else if (vtpci_with_feature(hw, VIRTIO_NET_F_MAC)) - virtio_set_hwaddr(hw); + return virtio_send_command(hw->cvq, &ctrl, &len, 1); + } + + if (!vtpci_with_feature(hw, VIRTIO_NET_F_MAC)) + return -ENOTSUP; + + virtio_set_hwaddr(hw); + return 0; } static int diff --git a/drivers/net/vmxnet3/vmxnet3_ethdev.c b/drivers/net/vmxnet3/vmxnet3_ethdev.c index 4e68aae6b..13ee8f250 100644 --- a/drivers/net/vmxnet3/vmxnet3_ethdev.c +++ b/drivers/net/vmxnet3/vmxnet3_ethdev.c @@ -73,7 +73,7 @@ vmxnet3_dev_supported_ptypes_get(struct rte_eth_dev *dev); static int vmxnet3_dev_vlan_filter_set(struct rte_eth_dev *dev, uint16_t vid, int on); static int vmxnet3_dev_vlan_offload_set(struct rte_eth_dev *dev, int mask); -static void vmxnet3_mac_addr_set(struct rte_eth_dev *dev, +static int vmxnet3_mac_addr_set(struct rte_eth_dev *dev, struct ether_addr *mac_addr); static void vmxnet3_interrupt_handler(void *param); @@ -1117,13 +1117,14 @@ vmxnet3_dev_supported_ptypes_get(struct rte_eth_dev *dev) return NULL; } -static void +static int vmxnet3_mac_addr_set(struct rte_eth_dev *dev, struct ether_addr *mac_addr) { struct vmxnet3_hw *hw = dev->data->dev_private; ether_addr_copy(mac_addr, (struct ether_addr *)(hw->perm_addr)); vmxnet3_write_mac(hw, mac_addr->addr_bytes); + return 0; } /* return 0 means link status changed, -1 means not changed */ diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c index 0590f0c10..6a7330d92 100644 --- a/lib/librte_ether/rte_ethdev.c +++ b/lib/librte_ether/rte_ethdev.c @@ -3003,6 +3003,7 @@ int rte_eth_dev_default_mac_addr_set(uint16_t port_id, struct ether_addr *addr) { struct rte_eth_dev *dev; + int ret; RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV); @@ -3012,11 +3013,13 @@ rte_eth_dev_default_mac_addr_set(uint16_t port_id, struct ether_addr *addr) dev = &rte_eth_devices[port_id]; RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->mac_addr_set, -ENOTSUP); + ret = (*dev->dev_ops->mac_addr_set)(dev, addr); + if (ret < 0) + return ret; + /* Update default address in NIC data structure */ ether_addr_copy(addr, &dev->data->mac_addrs[0]); - (*dev->dev_ops->mac_addr_set)(dev, addr); - return 0; } diff --git a/lib/librte_ether/rte_ethdev_core.h b/lib/librte_ether/rte_ethdev_core.h index e5681e466..55eb2b09e 100644 --- a/lib/librte_ether/rte_ethdev_core.h +++ b/lib/librte_ether/rte_ethdev_core.h @@ -255,7 +255,7 @@ typedef int (*eth_mac_addr_add_t)(struct rte_eth_dev *dev, uint32_t vmdq); /**< @internal Set a MAC address into Receive Address Address Register */ -typedef void (*eth_mac_addr_set_t)(struct rte_eth_dev *dev, +typedef int (*eth_mac_addr_set_t)(struct rte_eth_dev *dev, struct ether_addr *mac_addr); /**< @internal Set a MAC address into Receive Address Address Register */ diff --git a/test/test/virtual_pmd.c b/test/test/virtual_pmd.c index 2f5b31dba..69b4ba034 100644 --- a/test/test/virtual_pmd.c +++ b/test/test/virtual_pmd.c @@ -216,10 +216,11 @@ static void virtual_ethdev_promiscuous_mode_disable(struct rte_eth_dev *dev __rte_unused) {} -static void +static int virtual_ethdev_mac_address_set(__rte_unused struct rte_eth_dev *dev, __rte_unused struct ether_addr *addr) { + return 0; } static const struct eth_dev_ops virtual_ethdev_default_dev_ops = { -- 2.11.0