* [dpdk-dev] [PATCH] ethdev: modifiy vlan_offload_set_t to return int @ 2017-08-24 23:18 David Harton 2017-08-24 23:37 ` Stephen Hemminger 2017-08-25 13:33 ` [dpdk-dev] [PATCH v2] " David Harton 0 siblings, 2 replies; 19+ messages in thread From: David Harton @ 2017-08-24 23:18 UTC (permalink / raw) To: thomas, ferruh.yigit, stephen.hurd, ajit.khaparde, johndale, wenzhuo.lu, konstantin.ananyev, jingjing.wu, beilei.xing, jing.d.chen, adrien.mazarguil, nelio.laranjeiro, alejandro.lucero, hemant.agrawal, rasesh.mody, harish.patil, skhare, yliu, maxime.coquelin, allain.legacy Cc: dev, David Harton Some devices may not support or fail setting VLAN offload configuration based on dynamic circurmstances so the vlan_offload_set_t vector is modified to return an int so the caller can determine success or not. rte_eth_dev_set_vlan_offload is updated to return the value provided by the vector when called along with restoring the original offload configs on failure. Existing vlan_offload_set_t vectors are modified to return an int. Majority of cases return 0 but a few that actually can fail now return their failure codes. Finally, a vlan_offload_set_t vector is added to virtio to facilitate dynamically turning VLAN strip on or off. This is an ABI breakage that has been previously negotiated with Thomas and the proposed rel note change is included as well. Signed-off-by: David Harton <dharton@cisco.com> --- doc/guides/rel_notes/release_17_11.rst | 2 +- drivers/net/avp/avp_ethdev.c | 7 ++++--- drivers/net/bnxt/bnxt_ethdev.c | 9 ++++++--- drivers/net/dpaa2/dpaa2_ethdev.c | 13 ++++++++++--- drivers/net/e1000/em_ethdev.c | 12 +++++++++--- drivers/net/e1000/igb_ethdev.c | 12 +++++++++--- drivers/net/enic/enic_ethdev.c | 8 +++++--- drivers/net/fm10k/fm10k_ethdev.c | 3 ++- drivers/net/i40e/i40e_ethdev.c | 11 ++++++++--- drivers/net/i40e/i40e_ethdev_vf.c | 9 ++++++--- drivers/net/ixgbe/ixgbe_ethdev.c | 14 ++++++++------ drivers/net/mlx5/mlx5.h | 2 +- drivers/net/mlx5/mlx5_vlan.c | 3 ++- drivers/net/nfp/nfp_net.c | 13 +++++++------ drivers/net/qede/qede_ethdev.c | 6 ++++-- drivers/net/virtio/virtio_ethdev.c | 21 +++++++++++++++++++++ drivers/net/vmxnet3/vmxnet3_ethdev.c | 8 +++++--- lib/librte_ether/rte_ethdev.c | 14 +++++++++++++- lib/librte_ether/rte_ethdev.h | 2 +- 19 files changed, 122 insertions(+), 47 deletions(-) diff --git a/doc/guides/rel_notes/release_17_11.rst b/doc/guides/rel_notes/release_17_11.rst index 170f4f9..e74f70c 100644 --- a/doc/guides/rel_notes/release_17_11.rst +++ b/doc/guides/rel_notes/release_17_11.rst @@ -124,7 +124,7 @@ ABI Changes Also, make sure to start the actual text at the margin. ========================================================= - +* Changed return type of ``vlan_offload_set_t`` from ``void`` to ``int``. Shared Library Versions ----------------------- diff --git a/drivers/net/avp/avp_ethdev.c b/drivers/net/avp/avp_ethdev.c index c746a0e..3525e8f 100644 --- a/drivers/net/avp/avp_ethdev.c +++ b/drivers/net/avp/avp_ethdev.c @@ -70,7 +70,7 @@ static int avp_dev_create(struct rte_pci_device *pci_dev, static void avp_dev_close(struct rte_eth_dev *dev); static void avp_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info); -static void avp_vlan_offload_set(struct rte_eth_dev *dev, int mask); +static int avp_vlan_offload_set(struct rte_eth_dev *dev, int mask); static int avp_dev_link_update(struct rte_eth_dev *dev, int wait_to_complete); static void avp_dev_promiscuous_enable(struct rte_eth_dev *dev); static void avp_dev_promiscuous_disable(struct rte_eth_dev *dev); @@ -2031,7 +2031,7 @@ struct avp_queue { mask = (ETH_VLAN_STRIP_MASK | ETH_VLAN_FILTER_MASK | ETH_VLAN_EXTEND_MASK); - avp_vlan_offload_set(eth_dev, mask); + (void)avp_vlan_offload_set(eth_dev, mask); /* update device config */ memset(&config, 0, sizeof(config)); @@ -2214,7 +2214,7 @@ struct avp_queue { } } -static void +static int avp_vlan_offload_set(struct rte_eth_dev *eth_dev, int mask) { struct avp_dev *avp = AVP_DEV_PRIVATE_TO_HW(eth_dev->data->dev_private); @@ -2239,6 +2239,7 @@ struct avp_queue { if (eth_dev->data->dev_conf.rxmode.hw_vlan_extend) PMD_DRV_LOG(ERR, "VLAN extend offload not supported\n"); } + return 0; } static void diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c index c9d1122..547bd55 100644 --- a/drivers/net/bnxt/bnxt_ethdev.c +++ b/drivers/net/bnxt/bnxt_ethdev.c @@ -144,7 +144,7 @@ ETH_RSS_NONFRAG_IPV6_TCP | \ ETH_RSS_NONFRAG_IPV6_UDP) -static void bnxt_vlan_offload_set_op(struct rte_eth_dev *dev, int mask); +static int bnxt_vlan_offload_set_op(struct rte_eth_dev *dev, int mask); /***********************/ @@ -522,7 +522,9 @@ static int bnxt_dev_start_op(struct rte_eth_dev *eth_dev) vlan_mask |= ETH_VLAN_FILTER_MASK; if (eth_dev->data->dev_conf.rxmode.hw_vlan_strip) vlan_mask |= ETH_VLAN_STRIP_MASK; - bnxt_vlan_offload_set_op(eth_dev, vlan_mask); + rc = bnxt_vlan_offload_set_op(eth_dev, vlan_mask); + if (rc) + goto error; return 0; @@ -1275,7 +1277,7 @@ static int bnxt_vlan_filter_set_op(struct rte_eth_dev *eth_dev, return bnxt_del_vlan_filter(bp, vlan_id); } -static void +static int bnxt_vlan_offload_set_op(struct rte_eth_dev *dev, int mask) { struct bnxt *bp = (struct bnxt *)dev->data->dev_private; @@ -1307,6 +1309,7 @@ static int bnxt_vlan_filter_set_op(struct rte_eth_dev *eth_dev, if (mask & ETH_VLAN_EXTEND_MASK) RTE_LOG(ERR, PMD, "Extend VLAN Not supported\n"); + return 0; } static void diff --git a/drivers/net/dpaa2/dpaa2_ethdev.c b/drivers/net/dpaa2/dpaa2_ethdev.c index 429b3a0..3390cb3 100644 --- a/drivers/net/dpaa2/dpaa2_ethdev.c +++ b/drivers/net/dpaa2/dpaa2_ethdev.c @@ -138,7 +138,7 @@ return ret; } -static void +static int dpaa2_vlan_offload_set(struct rte_eth_dev *dev, int mask) { struct dpaa2_dev_priv *priv = dev->data->dev_private; @@ -158,6 +158,7 @@ RTE_LOG(ERR, PMD, "Unable to set vlan filter = %d\n", ret); } + return 0; } static int @@ -643,8 +644,14 @@ return ret; } /* VLAN Offload Settings */ - if (priv->max_vlan_filters) - dpaa2_vlan_offload_set(dev, ETH_VLAN_FILTER_MASK); + if (priv->max_vlan_filters) { + ret = dpaa2_vlan_offload_set(dev, ETH_VLAN_FILTER_MASK); + if (ret) { + PMD_INIT_LOG(ERR, "Error to dpaa2_vlan_offload_set:" + "code = %d\n", ret); + return ret; + } + } return 0; } diff --git a/drivers/net/e1000/em_ethdev.c b/drivers/net/e1000/em_ethdev.c index 3d4ab93..51f49d8 100644 --- a/drivers/net/e1000/em_ethdev.c +++ b/drivers/net/e1000/em_ethdev.c @@ -99,7 +99,7 @@ static int eth_em_interrupt_action(struct rte_eth_dev *dev, static int eth_em_vlan_filter_set(struct rte_eth_dev *dev, uint16_t vlan_id, int on); -static void eth_em_vlan_offload_set(struct rte_eth_dev *dev, int mask); +static int eth_em_vlan_offload_set(struct rte_eth_dev *dev, int mask); static void em_vlan_hw_filter_enable(struct rte_eth_dev *dev); static void em_vlan_hw_filter_disable(struct rte_eth_dev *dev); static void em_vlan_hw_strip_enable(struct rte_eth_dev *dev); @@ -668,7 +668,12 @@ static int eth_em_pci_remove(struct rte_pci_device *pci_dev) mask = ETH_VLAN_STRIP_MASK | ETH_VLAN_FILTER_MASK | \ ETH_VLAN_EXTEND_MASK; - eth_em_vlan_offload_set(dev, mask); + ret = eth_em_vlan_offload_set(dev, mask); + if (ret) { + PMD_INIT_LOG(ERR, "Unable to update vlan offload"); + em_dev_clear_queues(dev); + return ret; + } /* Set Interrupt Throttling Rate to maximum allowed value. */ E1000_WRITE_REG(hw, E1000_ITR, UINT16_MAX); @@ -1447,7 +1452,7 @@ static int eth_em_pci_remove(struct rte_pci_device *pci_dev) E1000_WRITE_REG(hw, E1000_CTRL, reg); } -static void +static int eth_em_vlan_offload_set(struct rte_eth_dev *dev, int mask) { if(mask & ETH_VLAN_STRIP_MASK){ @@ -1463,6 +1468,7 @@ static int eth_em_pci_remove(struct rte_pci_device *pci_dev) else em_vlan_hw_filter_disable(dev); } + return 0; } /* diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c index e4f7a9f..fa15ee9 100644 --- a/drivers/net/e1000/igb_ethdev.c +++ b/drivers/net/e1000/igb_ethdev.c @@ -157,7 +157,7 @@ static int eth_igb_vlan_filter_set(struct rte_eth_dev *dev, static int eth_igb_vlan_tpid_set(struct rte_eth_dev *dev, enum rte_vlan_type vlan_type, uint16_t tpid_id); -static void eth_igb_vlan_offload_set(struct rte_eth_dev *dev, int mask); +static int eth_igb_vlan_offload_set(struct rte_eth_dev *dev, int mask); static void igb_vlan_hw_filter_enable(struct rte_eth_dev *dev); static void igb_vlan_hw_filter_disable(struct rte_eth_dev *dev); @@ -1400,7 +1400,12 @@ static int eth_igbvf_pci_remove(struct rte_pci_device *pci_dev) */ mask = ETH_VLAN_STRIP_MASK | ETH_VLAN_FILTER_MASK | \ ETH_VLAN_EXTEND_MASK; - eth_igb_vlan_offload_set(dev, mask); + ret = eth_igb_vlan_offload_set(dev, mask); + if (ret) { + PMD_INIT_LOG(ERR, "Unable to set vlan offload"); + igb_dev_clear_queues(dev); + return ret; + } if (dev->data->dev_conf.rxmode.mq_mode == ETH_MQ_RX_VMDQ_ONLY) { /* Enable VLAN filter since VMDq always use VLAN filter */ @@ -2715,7 +2720,7 @@ static int eth_igbvf_xstats_get_names(__rte_unused struct rte_eth_dev *dev, 2 * VLAN_TAG_SIZE); } -static void +static int eth_igb_vlan_offload_set(struct rte_eth_dev *dev, int mask) { if(mask & ETH_VLAN_STRIP_MASK){ @@ -2738,6 +2743,7 @@ static int eth_igbvf_xstats_get_names(__rte_unused struct rte_eth_dev *dev, else igb_vlan_hw_extend_disable(dev); } + return 0; } diff --git a/drivers/net/enic/enic_ethdev.c b/drivers/net/enic/enic_ethdev.c index da8fec2..fc1eac2 100644 --- a/drivers/net/enic/enic_ethdev.c +++ b/drivers/net/enic/enic_ethdev.c @@ -347,7 +347,7 @@ static int enicpmd_vlan_filter_set(struct rte_eth_dev *eth_dev, return err; } -static void enicpmd_vlan_offload_set(struct rte_eth_dev *eth_dev, int mask) +static int enicpmd_vlan_offload_set(struct rte_eth_dev *eth_dev, int mask) { struct enic *enic = pmd_priv(eth_dev); @@ -371,6 +371,8 @@ static void enicpmd_vlan_offload_set(struct rte_eth_dev *eth_dev, int mask) dev_warning(enic, "Configuration of extended VLAN is not supported\n"); } + + return 0; } static int enicpmd_dev_configure(struct rte_eth_dev *eth_dev) @@ -392,9 +394,9 @@ static int enicpmd_dev_configure(struct rte_eth_dev *eth_dev) eth_dev->data->dev_conf.rxmode.split_hdr_size); } - enicpmd_vlan_offload_set(eth_dev, ETH_VLAN_STRIP_MASK); + ret = enicpmd_vlan_offload_set(eth_dev, ETH_VLAN_STRIP_MASK); enic->hw_ip_checksum = eth_dev->data->dev_conf.rxmode.hw_ip_checksum; - return 0; + return ret; } /* Start the device. diff --git a/drivers/net/fm10k/fm10k_ethdev.c b/drivers/net/fm10k/fm10k_ethdev.c index e60d3a3..f4626f7 100644 --- a/drivers/net/fm10k/fm10k_ethdev.c +++ b/drivers/net/fm10k/fm10k_ethdev.c @@ -1590,7 +1590,7 @@ static int fm10k_xstats_get_names(__rte_unused struct rte_eth_dev *dev, return 0; } -static void +static int fm10k_vlan_offload_set(struct rte_eth_dev *dev, int mask) { if (mask & ETH_VLAN_STRIP_MASK) { @@ -1609,6 +1609,7 @@ static int fm10k_xstats_get_names(__rte_unused struct rte_eth_dev *dev, if (!dev->data->dev_conf.rxmode.hw_vlan_filter) PMD_INIT_LOG(ERR, "VLAN filter is always on in fm10k"); } + return 0; } /* Add/Remove a MAC address, and update filters to main VSI */ diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c index 00b6082..d03a44b 100644 --- a/drivers/net/i40e/i40e_ethdev.c +++ b/drivers/net/i40e/i40e_ethdev.c @@ -278,7 +278,7 @@ static int i40e_vlan_filter_set(struct rte_eth_dev *dev, static int i40e_vlan_tpid_set(struct rte_eth_dev *dev, enum rte_vlan_type vlan_type, uint16_t tpid); -static void i40e_vlan_offload_set(struct rte_eth_dev *dev, int mask); +static int i40e_vlan_offload_set(struct rte_eth_dev *dev, int mask); static void i40e_vlan_strip_queue_set(struct rte_eth_dev *dev, uint16_t queue, int on); @@ -3130,7 +3130,7 @@ static int i40e_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev, return ret; } -static void +static int i40e_vlan_offload_set(struct rte_eth_dev *dev, int mask) { struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private); @@ -3163,6 +3163,7 @@ static int i40e_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev, else i40e_vsi_config_double_vlan(vsi, FALSE); } + return 0; } static void @@ -5216,7 +5217,11 @@ struct i40e_vsi * /* Apply vlan offload setting */ mask = ETH_VLAN_STRIP_MASK | ETH_VLAN_FILTER_MASK; - i40e_vlan_offload_set(dev, mask); + ret = i40e_vlan_offload_set(dev, mask); + if (ret) { + PMD_DRV_LOG(INFO, "Failed to update vlan offload"); + return ret; + } /* Apply double-vlan setting, not implemented yet */ diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c index f6d8293..f7fffc2 100644 --- a/drivers/net/i40e/i40e_ethdev_vf.c +++ b/drivers/net/i40e/i40e_ethdev_vf.c @@ -118,7 +118,7 @@ static int i40evf_dev_xstats_get_names(struct rte_eth_dev *dev, static void i40evf_dev_xstats_reset(struct rte_eth_dev *dev); static int i40evf_vlan_filter_set(struct rte_eth_dev *dev, uint16_t vlan_id, int on); -static void i40evf_vlan_offload_set(struct rte_eth_dev *dev, int mask); +static int i40evf_vlan_offload_set(struct rte_eth_dev *dev, int mask); static int i40evf_vlan_pvid_set(struct rte_eth_dev *dev, uint16_t pvid, int on); static void i40evf_dev_close(struct rte_eth_dev *dev); @@ -1634,7 +1634,9 @@ static int eth_i40evf_pci_remove(struct rte_pci_device *pci_dev) int ret; /* Apply vlan offload setting */ - i40evf_vlan_offload_set(dev, ETH_VLAN_STRIP_MASK); + ret = i40evf_vlan_offload_set(dev, ETH_VLAN_STRIP_MASK); + if (ret) + return ret; /* Apply pvid setting */ ret = i40evf_vlan_pvid_set(dev, data->dev_conf.txmode.pvid, @@ -1642,7 +1644,7 @@ static int eth_i40evf_pci_remove(struct rte_pci_device *pci_dev) return ret; } -static void +static int i40evf_vlan_offload_set(struct rte_eth_dev *dev, int mask) { struct rte_eth_conf *dev_conf = &dev->data->dev_conf; @@ -1655,6 +1657,7 @@ static int eth_i40evf_pci_remove(struct rte_pci_device *pci_dev) else i40evf_disable_vlan_strip(dev); } + return 0; } static int diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c index 22171d8..043a713 100644 --- a/drivers/net/ixgbe/ixgbe_ethdev.c +++ b/drivers/net/ixgbe/ixgbe_ethdev.c @@ -218,7 +218,7 @@ static void ixgbe_vlan_hw_strip_bitmap_set(struct rte_eth_dev *dev, uint16_t queue, bool on); static void ixgbe_vlan_strip_queue_set(struct rte_eth_dev *dev, uint16_t queue, int on); -static void ixgbe_vlan_offload_set(struct rte_eth_dev *dev, int mask); +static int ixgbe_vlan_offload_set(struct rte_eth_dev *dev, int mask); static void ixgbe_vlan_hw_strip_enable(struct rte_eth_dev *dev, uint16_t queue); static void ixgbe_vlan_hw_strip_disable(struct rte_eth_dev *dev, uint16_t queue); static void ixgbe_vlan_hw_extend_enable(struct rte_eth_dev *dev); @@ -274,7 +274,7 @@ static int ixgbevf_vlan_filter_set(struct rte_eth_dev *dev, uint16_t vlan_id, int on); static void ixgbevf_vlan_strip_queue_set(struct rte_eth_dev *dev, uint16_t queue, int on); -static void ixgbevf_vlan_offload_set(struct rte_eth_dev *dev, int mask); +static int ixgbevf_vlan_offload_set(struct rte_eth_dev *dev, int mask); static void ixgbevf_set_vfta_all(struct rte_eth_dev *dev, bool on); static int ixgbevf_dev_rx_queue_intr_enable(struct rte_eth_dev *dev, uint16_t queue_id); @@ -2125,7 +2125,7 @@ static int eth_ixgbevf_pci_remove(struct rte_pci_device *pci_dev) */ } -static void +static int ixgbe_vlan_offload_set(struct rte_eth_dev *dev, int mask) { if (mask & ETH_VLAN_STRIP_MASK) { @@ -2148,6 +2148,7 @@ static int eth_ixgbevf_pci_remove(struct rte_pci_device *pci_dev) else ixgbe_vlan_hw_extend_disable(dev); } + return 0; } static void @@ -2570,7 +2571,7 @@ static int eth_ixgbevf_pci_remove(struct rte_pci_device *pci_dev) mask = ETH_VLAN_STRIP_MASK | ETH_VLAN_FILTER_MASK | ETH_VLAN_EXTEND_MASK; - ixgbe_vlan_offload_set(dev, mask); + (void)ixgbe_vlan_offload_set(dev, mask); if (dev->data->dev_conf.rxmode.mq_mode == ETH_MQ_RX_VMDQ_ONLY) { /* Enable vlan filtering for VMDq */ @@ -4993,7 +4994,7 @@ static int ixgbevf_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev, /* Set HW strip */ mask = ETH_VLAN_STRIP_MASK | ETH_VLAN_FILTER_MASK | ETH_VLAN_EXTEND_MASK; - ixgbevf_vlan_offload_set(dev, mask); + (void)ixgbevf_vlan_offload_set(dev, mask); ixgbevf_dev_rxtx_start(dev); @@ -5153,7 +5154,7 @@ static void ixgbevf_set_vfta_all(struct rte_eth_dev *dev, bool on) ixgbe_vlan_hw_strip_bitmap_set(dev, queue, on); } -static void +static int ixgbevf_vlan_offload_set(struct rte_eth_dev *dev, int mask) { struct ixgbe_hw *hw = @@ -5168,6 +5169,7 @@ static void ixgbevf_set_vfta_all(struct rte_eth_dev *dev, bool on) for (i = 0; i < hw->mac.max_rx_queues; i++) ixgbevf_vlan_strip_queue_set(dev, i, on); } + return 0; } int diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h index 43c5384..93e71be 100644 --- a/drivers/net/mlx5/mlx5.h +++ b/drivers/net/mlx5/mlx5.h @@ -283,7 +283,7 @@ int mlx5_xstats_get_names(struct rte_eth_dev *, /* mlx5_vlan.c */ int mlx5_vlan_filter_set(struct rte_eth_dev *, uint16_t, int); -void mlx5_vlan_offload_set(struct rte_eth_dev *, int); +int mlx5_vlan_offload_set(struct rte_eth_dev *, int); void mlx5_vlan_strip_queue_set(struct rte_eth_dev *, uint16_t, int); /* mlx5_trigger.c */ diff --git a/drivers/net/mlx5/mlx5_vlan.c b/drivers/net/mlx5/mlx5_vlan.c index 1b0fa40..7215909 100644 --- a/drivers/net/mlx5/mlx5_vlan.c +++ b/drivers/net/mlx5/mlx5_vlan.c @@ -210,7 +210,7 @@ * @param mask * VLAN offload bit mask. */ -void +int mlx5_vlan_offload_set(struct rte_eth_dev *dev, int mask) { struct priv *priv = dev->data->dev_private; @@ -230,4 +230,5 @@ priv_vlan_strip_queue_set(priv, i, hw_vlan_strip); priv_unlock(priv); } + return 0; } diff --git a/drivers/net/nfp/nfp_net.c b/drivers/net/nfp/nfp_net.c index 92b03c4..6473edc 100644 --- a/drivers/net/nfp/nfp_net.c +++ b/drivers/net/nfp/nfp_net.c @@ -2149,11 +2149,12 @@ uint32_t nfp_net_txq_full(struct nfp_net_txq *txq) return i; } -static void +static int nfp_net_vlan_offload_set(struct rte_eth_dev *dev, int mask) { uint32_t new_ctrl, update; struct nfp_net_hw *hw; + int ret; hw = NFP_NET_DEV_PRIVATE_TO_HW(dev->data->dev_private); new_ctrl = 0; @@ -2174,14 +2175,14 @@ uint32_t nfp_net_txq_full(struct nfp_net_txq *txq) new_ctrl = hw->ctrl & ~NFP_NET_CFG_CTRL_RXVLAN; if (new_ctrl == 0) - return; + return 0; update = NFP_NET_CFG_UPDATE_GEN; - if (nfp_net_reconfig(hw, new_ctrl, update) < 0) - return; - - hw->ctrl = new_ctrl; + ret = nfp_net_reconfig(hw, new_ctrl, update); + if (!ret) + hw->ctrl = new_ctrl; + return ret; } /* Update Redirection Table(RETA) of Receive Side Scaling of Ethernet device */ diff --git a/drivers/net/qede/qede_ethdev.c b/drivers/net/qede/qede_ethdev.c index 0e05989..ee29754 100644 --- a/drivers/net/qede/qede_ethdev.c +++ b/drivers/net/qede/qede_ethdev.c @@ -975,7 +975,7 @@ static int qede_vlan_filter_set(struct rte_eth_dev *eth_dev, return rc; } -static void qede_vlan_offload_set(struct rte_eth_dev *eth_dev, int mask) +static int qede_vlan_offload_set(struct rte_eth_dev *eth_dev, int mask) { struct qede_dev *qdev = QEDE_INIT_QDEV(eth_dev); struct ecore_dev *edev = QEDE_INIT_EDEV(qdev); @@ -1013,6 +1013,8 @@ static void qede_vlan_offload_set(struct rte_eth_dev *eth_dev, int mask) DP_INFO(edev, "vlan offload mask %d vlan-strip %d vlan-filter %d\n", mask, rxmode->hw_vlan_strip, rxmode->hw_vlan_filter); + + return 0; } static void qede_prandom_bytes(uint32_t *buff) @@ -1237,7 +1239,7 @@ static int qede_dev_configure(struct rte_eth_dev *eth_dev) qdev->enable_lro = rxmode->enable_lro; /* Enable VLAN offloads by default */ - qede_vlan_offload_set(eth_dev, ETH_VLAN_STRIP_MASK | + (void)qede_vlan_offload_set(eth_dev, ETH_VLAN_STRIP_MASK | ETH_VLAN_FILTER_MASK | ETH_VLAN_EXTEND_MASK); diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c index e320811..fba92f4 100644 --- a/drivers/net/virtio/virtio_ethdev.c +++ b/drivers/net/virtio/virtio_ethdev.c @@ -72,6 +72,7 @@ static void virtio_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info); static int virtio_dev_link_update(struct rte_eth_dev *dev, int wait_to_complete); +static int virtio_dev_vlan_offload_set(struct rte_eth_dev *dev, int mask); static void virtio_set_hwaddr(struct virtio_hw *hw); static void virtio_get_hwaddr(struct virtio_hw *hw); @@ -779,6 +780,7 @@ struct rte_virtio_xstats_name_off { .stats_reset = virtio_dev_stats_reset, .xstats_reset = virtio_dev_stats_reset, .link_update = virtio_dev_link_update, + .vlan_offload_set = virtio_dev_vlan_offload_set, .rx_queue_setup = virtio_dev_rx_queue_setup, .rx_queue_intr_enable = virtio_dev_rx_queue_intr_enable, .rx_queue_intr_disable = virtio_dev_rx_queue_intr_disable, @@ -1875,6 +1877,25 @@ static void virtio_dev_free_mbufs(struct rte_eth_dev *dev) return (old.link_status == link.link_status) ? -1 : 0; } +static int +virtio_dev_vlan_offload_set(struct rte_eth_dev *dev, int mask) +{ + const struct rte_eth_rxmode *rxmode = &dev->data->dev_conf.rxmode; + struct virtio_hw *hw = dev->data->dev_private; + + if (rxmode->hw_vlan_filter + && !vtpci_with_feature(hw, VIRTIO_NET_F_CTRL_VLAN)) { + PMD_DRV_LOG(NOTICE, + "vlan filtering not available on this host"); + return -ENOTSUP; + } + + if (mask & ETH_VLAN_STRIP_MASK) + hw->vlan_strip = rxmode->hw_vlan_strip; + + return 0; +} + static void virtio_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info) { diff --git a/drivers/net/vmxnet3/vmxnet3_ethdev.c b/drivers/net/vmxnet3/vmxnet3_ethdev.c index 3910991..81ee262 100644 --- a/drivers/net/vmxnet3/vmxnet3_ethdev.c +++ b/drivers/net/vmxnet3/vmxnet3_ethdev.c @@ -100,7 +100,7 @@ static void vmxnet3_dev_info_get(struct rte_eth_dev *dev, 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 void vmxnet3_dev_vlan_offload_set(struct rte_eth_dev *dev, int mask); +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, struct ether_addr *mac_addr); static void vmxnet3_interrupt_handler(void *param); @@ -730,7 +730,7 @@ static int eth_vmxnet3_pci_remove(struct rte_pci_device *pci_dev) devRead->rssConfDesc.confPA = hw->rss_confPA; } - vmxnet3_dev_vlan_offload_set(dev, + (void)vmxnet3_dev_vlan_offload_set(dev, ETH_VLAN_STRIP_MASK | ETH_VLAN_FILTER_MASK); vmxnet3_write_mac(hw, dev->data->mac_addrs->addr_bytes); @@ -1275,7 +1275,7 @@ static int eth_vmxnet3_pci_remove(struct rte_pci_device *pci_dev) return 0; } -static void +static int vmxnet3_dev_vlan_offload_set(struct rte_eth_dev *dev, int mask) { struct vmxnet3_hw *hw = dev->data->dev_private; @@ -1301,6 +1301,8 @@ static int eth_vmxnet3_pci_remove(struct rte_pci_device *pci_dev) VMXNET3_WRITE_BAR1_REG(hw, VMXNET3_REG_CMD, VMXNET3_CMD_UPDATE_VLAN_FILTERS); } + + return 0; } static void diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c index 0597641..f13f813 100644 --- a/lib/librte_ether/rte_ethdev.c +++ b/lib/librte_ether/rte_ethdev.c @@ -2049,10 +2049,16 @@ struct rte_eth_dev * int ret = 0; int mask = 0; int cur, org = 0; + uint8_t org_strip, org_filter, org_extend; RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV); dev = &rte_eth_devices[port_id]; + /* save original values in case of failure */ + org_strip = dev->data->dev_conf.rxmode.hw_vlan_strip; + org_filter = dev->data->dev_conf.rxmode.hw_vlan_filter; + org_extend = dev->data->dev_conf.rxmode.hw_vlan_extend; + /*check which option changed by application*/ cur = !!(offload_mask & ETH_VLAN_STRIP_OFFLOAD); org = !!(dev->data->dev_conf.rxmode.hw_vlan_strip); @@ -2080,7 +2086,13 @@ struct rte_eth_dev * return ret; RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->vlan_offload_set, -ENOTSUP); - (*dev->dev_ops->vlan_offload_set)(dev, mask); + ret = (*dev->dev_ops->vlan_offload_set)(dev, mask); + if (ret) { + /* hit an error restore original values */ + dev->data->dev_conf.rxmode.hw_vlan_strip = org_strip; + dev->data->dev_conf.rxmode.hw_vlan_filter = org_filter; + dev->data->dev_conf.rxmode.hw_vlan_extend = org_extend; + } return ret; } diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h index 0adf327..7254fd0 100644 --- a/lib/librte_ether/rte_ethdev.h +++ b/lib/librte_ether/rte_ethdev.h @@ -1245,7 +1245,7 @@ typedef int (*vlan_tpid_set_t)(struct rte_eth_dev *dev, enum rte_vlan_type type, uint16_t tpid); /**< @internal set the outer/inner VLAN-TPID by an Ethernet device. */ -typedef void (*vlan_offload_set_t)(struct rte_eth_dev *dev, int mask); +typedef int (*vlan_offload_set_t)(struct rte_eth_dev *dev, int mask); /**< @internal set VLAN offload function by an Ethernet device. */ typedef int (*vlan_pvid_set_t)(struct rte_eth_dev *dev, -- 1.8.3.1 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [dpdk-dev] [PATCH] ethdev: modifiy vlan_offload_set_t to return int 2017-08-24 23:18 [dpdk-dev] [PATCH] ethdev: modifiy vlan_offload_set_t to return int David Harton @ 2017-08-24 23:37 ` Stephen Hemminger 2017-08-25 0:55 ` David Harton (dharton) 2017-08-25 13:33 ` [dpdk-dev] [PATCH v2] " David Harton 1 sibling, 1 reply; 19+ messages in thread From: Stephen Hemminger @ 2017-08-24 23:37 UTC (permalink / raw) To: David Harton Cc: thomas, ferruh.yigit, stephen.hurd, ajit.khaparde, johndale, wenzhuo.lu, konstantin.ananyev, jingjing.wu, beilei.xing, jing.d.chen, adrien.mazarguil, nelio.laranjeiro, alejandro.lucero, hemant.agrawal, rasesh.mody, harish.patil, skhare, yliu, maxime.coquelin, allain.legacy, dev On Thu, 24 Aug 2017 19:18:51 -0400 David Harton <dharton@cisco.com> wrote: > @@ -2031,7 +2031,7 @@ struct avp_queue { > mask = (ETH_VLAN_STRIP_MASK | > ETH_VLAN_FILTER_MASK | > ETH_VLAN_EXTEND_MASK); > - avp_vlan_offload_set(eth_dev, mask); > + (void)avp_vlan_offload_set(eth_dev, mask); This is a BSDism. You don't need the void cast. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [dpdk-dev] [PATCH] ethdev: modifiy vlan_offload_set_t to return int 2017-08-24 23:37 ` Stephen Hemminger @ 2017-08-25 0:55 ` David Harton (dharton) 2017-08-25 8:20 ` Bruce Richardson 0 siblings, 1 reply; 19+ messages in thread From: David Harton (dharton) @ 2017-08-25 0:55 UTC (permalink / raw) To: Stephen Hemminger Cc: thomas, ferruh.yigit, stephen.hurd, ajit.khaparde, John Daley (johndale), wenzhuo.lu, konstantin.ananyev, jingjing.wu, beilei.xing, jing.d.chen, adrien.mazarguil, nelio.laranjeiro, alejandro.lucero, hemant.agrawal, rasesh.mody, harish.patil, skhare, yliu, maxime.coquelin, allain.legacy, dev > -----Original Message----- > From: Stephen Hemminger [mailto:stephen@networkplumber.org] > Sent: Thursday, August 24, 2017 7:37 PM > To: David Harton (dharton) <dharton@cisco.com> > Cc: thomas@monjalon.net; ferruh.yigit@intel.com; > stephen.hurd@broadcom.com; ajit.khaparde@broadcom.com; John Daley > (johndale) <johndale@cisco.com>; wenzhuo.lu@intel.com; > konstantin.ananyev@intel.com; jingjing.wu@intel.com; > beilei.xing@intel.com; jing.d.chen@intel.com; adrien.mazarguil@6wind.com; > nelio.laranjeiro@6wind.com; alejandro.lucero@netronome.com; > hemant.agrawal@nxp.com; rasesh.mody@cavium.com; harish.patil@cavium.com; > skhare@vmware.com; yliu@fridaylinux.org; maxime.coquelin@redhat.com; > allain.legacy@windriver.com; dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH] ethdev: modifiy vlan_offload_set_t to > return int > > On Thu, 24 Aug 2017 19:18:51 -0400 > David Harton <dharton@cisco.com> wrote: > > > @@ -2031,7 +2031,7 @@ struct avp_queue { > > mask = (ETH_VLAN_STRIP_MASK | > > ETH_VLAN_FILTER_MASK | > > ETH_VLAN_EXTEND_MASK); > > - avp_vlan_offload_set(eth_dev, mask); > > + (void)avp_vlan_offload_set(eth_dev, mask); > > This is a BSDism. You don't need the void cast. Never know what to do...SA tools and some compilers whine if I don't. People complain if I do. :) What if I check the return code and log an error? ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [dpdk-dev] [PATCH] ethdev: modifiy vlan_offload_set_t to return int 2017-08-25 0:55 ` David Harton (dharton) @ 2017-08-25 8:20 ` Bruce Richardson 0 siblings, 0 replies; 19+ messages in thread From: Bruce Richardson @ 2017-08-25 8:20 UTC (permalink / raw) To: David Harton (dharton) Cc: Stephen Hemminger, thomas, ferruh.yigit, stephen.hurd, ajit.khaparde, John Daley (johndale), wenzhuo.lu, konstantin.ananyev, jingjing.wu, beilei.xing, jing.d.chen, adrien.mazarguil, nelio.laranjeiro, alejandro.lucero, hemant.agrawal, rasesh.mody, harish.patil, skhare, yliu, maxime.coquelin, allain.legacy, dev On Fri, Aug 25, 2017 at 12:55:21AM +0000, David Harton (dharton) wrote: > > > > -----Original Message----- > > From: Stephen Hemminger [mailto:stephen@networkplumber.org] > > Sent: Thursday, August 24, 2017 7:37 PM > > To: David Harton (dharton) <dharton@cisco.com> > > Cc: thomas@monjalon.net; ferruh.yigit@intel.com; > > stephen.hurd@broadcom.com; ajit.khaparde@broadcom.com; John Daley > > (johndale) <johndale@cisco.com>; wenzhuo.lu@intel.com; > > konstantin.ananyev@intel.com; jingjing.wu@intel.com; > > beilei.xing@intel.com; jing.d.chen@intel.com; adrien.mazarguil@6wind.com; > > nelio.laranjeiro@6wind.com; alejandro.lucero@netronome.com; > > hemant.agrawal@nxp.com; rasesh.mody@cavium.com; harish.patil@cavium.com; > > skhare@vmware.com; yliu@fridaylinux.org; maxime.coquelin@redhat.com; > > allain.legacy@windriver.com; dev@dpdk.org > > Subject: Re: [dpdk-dev] [PATCH] ethdev: modifiy vlan_offload_set_t to > > return int > > > > On Thu, 24 Aug 2017 19:18:51 -0400 > > David Harton <dharton@cisco.com> wrote: > > > > > @@ -2031,7 +2031,7 @@ struct avp_queue { > > > mask = (ETH_VLAN_STRIP_MASK | > > > ETH_VLAN_FILTER_MASK | > > > ETH_VLAN_EXTEND_MASK); > > > - avp_vlan_offload_set(eth_dev, mask); > > > + (void)avp_vlan_offload_set(eth_dev, mask); > > > > This is a BSDism. You don't need the void cast. > > Never know what to do...SA tools and some compilers whine if I don't. > People complain if I do. :) > > What if I check the return code and log an error? Sounds like a case of "doing the right thing" to me :-) ^ permalink raw reply [flat|nested] 19+ messages in thread
* [dpdk-dev] [PATCH v2] ethdev: modifiy vlan_offload_set_t to return int 2017-08-24 23:18 [dpdk-dev] [PATCH] ethdev: modifiy vlan_offload_set_t to return int David Harton 2017-08-24 23:37 ` Stephen Hemminger @ 2017-08-25 13:33 ` David Harton 2017-08-25 13:47 ` [dpdk-dev] [PATCH v3] " David Harton 1 sibling, 1 reply; 19+ messages in thread From: David Harton @ 2017-08-25 13:33 UTC (permalink / raw) To: thomas, ferruh.yigit, stephen.hurd, ajit.khaparde, johndale, konstantin.ananyev, jingjing.wu, beilei.xing, jing.d.chen, adrien.mazarguil, nelio.laranjeiro, alejandro.lucero, hemant.agrawal, rasesh.mody, harish.patil, skhare, yliu, maxime.coquelin, allain.legacy Cc: dev, David Harton Some devices may not support or fail setting VLAN offload configuration based on dynamic circurmstances so the vlan_offload_set_t vector is modified to return an int so the caller can determine success or not. rte_eth_dev_set_vlan_offload is updated to return the value provided by the vector when called along with restoring the original offload configs on failure. Existing vlan_offload_set_t vectors are modified to return an int. Majority of cases return 0 but a few that actually can fail now return their failure codes. Finally, a vlan_offload_set_t vector is added to virtio to facilitate dynamically turning VLAN strip on or off. Signed-off-by: David Harton <dharton@cisco.com> --- *v2 Fixed a missed format error. Removed vlan offload vector call casts and replaced with checks for return values. *v1 This is an ABI breakage that has been previously negotiated with Thomas and the proposed rel note change is included as well. doc/guides/rel_notes/release_17_11.rst | 2 +- drivers/net/avp/avp_ethdev.c | 12 +++++++++--- drivers/net/bnxt/bnxt_ethdev.c | 9 ++++++--- drivers/net/dpaa2/dpaa2_ethdev.c | 13 ++++++++++--- drivers/net/e1000/em_ethdev.c | 12 +++++++++--- drivers/net/e1000/igb_ethdev.c | 12 +++++++++--- drivers/net/enic/enic_ethdev.c | 8 +++++--- drivers/net/fm10k/fm10k_ethdev.c | 3 ++- drivers/net/i40e/i40e_ethdev.c | 11 ++++++++--- drivers/net/i40e/i40e_ethdev_vf.c | 9 ++++++--- drivers/net/ixgbe/ixgbe_ethdev.c | 25 ++++++++++++++++++------- drivers/net/mlx5/mlx5.h | 2 +- drivers/net/mlx5/mlx5_vlan.c | 3 ++- drivers/net/nfp/nfp_net.c | 13 +++++++------ drivers/net/qede/qede_ethdev.c | 9 +++++++-- drivers/net/virtio/virtio_ethdev.c | 21 +++++++++++++++++++++ drivers/net/vmxnet3/vmxnet3_ethdev.c | 10 +++++++--- lib/librte_ether/rte_ethdev.c | 14 +++++++++++++- lib/librte_ether/rte_ethdev.h | 2 +- 19 files changed, 142 insertions(+), 48 deletions(-) diff --git a/doc/guides/rel_notes/release_17_11.rst b/doc/guides/rel_notes/release_17_11.rst index 170f4f9..e74f70c 100644 --- a/doc/guides/rel_notes/release_17_11.rst +++ b/doc/guides/rel_notes/release_17_11.rst @@ -124,7 +124,7 @@ ABI Changes Also, make sure to start the actual text at the margin. ========================================================= - +* Changed return type of ``vlan_offload_set_t`` from ``void`` to ``int``. Shared Library Versions ----------------------- diff --git a/drivers/net/avp/avp_ethdev.c b/drivers/net/avp/avp_ethdev.c index c746a0e..4011dfa 100644 --- a/drivers/net/avp/avp_ethdev.c +++ b/drivers/net/avp/avp_ethdev.c @@ -70,7 +70,7 @@ static int avp_dev_create(struct rte_pci_device *pci_dev, static void avp_dev_close(struct rte_eth_dev *dev); static void avp_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info); -static void avp_vlan_offload_set(struct rte_eth_dev *dev, int mask); +static int avp_vlan_offload_set(struct rte_eth_dev *dev, int mask); static int avp_dev_link_update(struct rte_eth_dev *dev, int wait_to_complete); static void avp_dev_promiscuous_enable(struct rte_eth_dev *dev); static void avp_dev_promiscuous_disable(struct rte_eth_dev *dev); @@ -2031,7 +2031,12 @@ struct avp_queue { mask = (ETH_VLAN_STRIP_MASK | ETH_VLAN_FILTER_MASK | ETH_VLAN_EXTEND_MASK); - avp_vlan_offload_set(eth_dev, mask); + ret = avp_vlan_offload_set(eth_dev, mask); + if (ret < 0) { + PMD_DRV_LOG(ERR, "VLAN offload set failed by host, ret=%d\n", + ret); + goto unlock; + } /* update device config */ memset(&config, 0, sizeof(config)); @@ -2214,7 +2219,7 @@ struct avp_queue { } } -static void +static int avp_vlan_offload_set(struct rte_eth_dev *eth_dev, int mask) { struct avp_dev *avp = AVP_DEV_PRIVATE_TO_HW(eth_dev->data->dev_private); @@ -2239,6 +2244,7 @@ struct avp_queue { if (eth_dev->data->dev_conf.rxmode.hw_vlan_extend) PMD_DRV_LOG(ERR, "VLAN extend offload not supported\n"); } + return 0; } static void diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c index c9d1122..547bd55 100644 --- a/drivers/net/bnxt/bnxt_ethdev.c +++ b/drivers/net/bnxt/bnxt_ethdev.c @@ -144,7 +144,7 @@ ETH_RSS_NONFRAG_IPV6_TCP | \ ETH_RSS_NONFRAG_IPV6_UDP) -static void bnxt_vlan_offload_set_op(struct rte_eth_dev *dev, int mask); +static int bnxt_vlan_offload_set_op(struct rte_eth_dev *dev, int mask); /***********************/ @@ -522,7 +522,9 @@ static int bnxt_dev_start_op(struct rte_eth_dev *eth_dev) vlan_mask |= ETH_VLAN_FILTER_MASK; if (eth_dev->data->dev_conf.rxmode.hw_vlan_strip) vlan_mask |= ETH_VLAN_STRIP_MASK; - bnxt_vlan_offload_set_op(eth_dev, vlan_mask); + rc = bnxt_vlan_offload_set_op(eth_dev, vlan_mask); + if (rc) + goto error; return 0; @@ -1275,7 +1277,7 @@ static int bnxt_vlan_filter_set_op(struct rte_eth_dev *eth_dev, return bnxt_del_vlan_filter(bp, vlan_id); } -static void +static int bnxt_vlan_offload_set_op(struct rte_eth_dev *dev, int mask) { struct bnxt *bp = (struct bnxt *)dev->data->dev_private; @@ -1307,6 +1309,7 @@ static int bnxt_vlan_filter_set_op(struct rte_eth_dev *eth_dev, if (mask & ETH_VLAN_EXTEND_MASK) RTE_LOG(ERR, PMD, "Extend VLAN Not supported\n"); + return 0; } static void diff --git a/drivers/net/dpaa2/dpaa2_ethdev.c b/drivers/net/dpaa2/dpaa2_ethdev.c index 429b3a0..3390cb3 100644 --- a/drivers/net/dpaa2/dpaa2_ethdev.c +++ b/drivers/net/dpaa2/dpaa2_ethdev.c @@ -138,7 +138,7 @@ return ret; } -static void +static int dpaa2_vlan_offload_set(struct rte_eth_dev *dev, int mask) { struct dpaa2_dev_priv *priv = dev->data->dev_private; @@ -158,6 +158,7 @@ RTE_LOG(ERR, PMD, "Unable to set vlan filter = %d\n", ret); } + return 0; } static int @@ -643,8 +644,14 @@ return ret; } /* VLAN Offload Settings */ - if (priv->max_vlan_filters) - dpaa2_vlan_offload_set(dev, ETH_VLAN_FILTER_MASK); + if (priv->max_vlan_filters) { + ret = dpaa2_vlan_offload_set(dev, ETH_VLAN_FILTER_MASK); + if (ret) { + PMD_INIT_LOG(ERR, "Error to dpaa2_vlan_offload_set:" + "code = %d\n", ret); + return ret; + } + } return 0; } diff --git a/drivers/net/e1000/em_ethdev.c b/drivers/net/e1000/em_ethdev.c index 3d4ab93..51f49d8 100644 --- a/drivers/net/e1000/em_ethdev.c +++ b/drivers/net/e1000/em_ethdev.c @@ -99,7 +99,7 @@ static int eth_em_interrupt_action(struct rte_eth_dev *dev, static int eth_em_vlan_filter_set(struct rte_eth_dev *dev, uint16_t vlan_id, int on); -static void eth_em_vlan_offload_set(struct rte_eth_dev *dev, int mask); +static int eth_em_vlan_offload_set(struct rte_eth_dev *dev, int mask); static void em_vlan_hw_filter_enable(struct rte_eth_dev *dev); static void em_vlan_hw_filter_disable(struct rte_eth_dev *dev); static void em_vlan_hw_strip_enable(struct rte_eth_dev *dev); @@ -668,7 +668,12 @@ static int eth_em_pci_remove(struct rte_pci_device *pci_dev) mask = ETH_VLAN_STRIP_MASK | ETH_VLAN_FILTER_MASK | \ ETH_VLAN_EXTEND_MASK; - eth_em_vlan_offload_set(dev, mask); + ret = eth_em_vlan_offload_set(dev, mask); + if (ret) { + PMD_INIT_LOG(ERR, "Unable to update vlan offload"); + em_dev_clear_queues(dev); + return ret; + } /* Set Interrupt Throttling Rate to maximum allowed value. */ E1000_WRITE_REG(hw, E1000_ITR, UINT16_MAX); @@ -1447,7 +1452,7 @@ static int eth_em_pci_remove(struct rte_pci_device *pci_dev) E1000_WRITE_REG(hw, E1000_CTRL, reg); } -static void +static int eth_em_vlan_offload_set(struct rte_eth_dev *dev, int mask) { if(mask & ETH_VLAN_STRIP_MASK){ @@ -1463,6 +1468,7 @@ static int eth_em_pci_remove(struct rte_pci_device *pci_dev) else em_vlan_hw_filter_disable(dev); } + return 0; } /* diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c index e4f7a9f..fa15ee9 100644 --- a/drivers/net/e1000/igb_ethdev.c +++ b/drivers/net/e1000/igb_ethdev.c @@ -157,7 +157,7 @@ static int eth_igb_vlan_filter_set(struct rte_eth_dev *dev, static int eth_igb_vlan_tpid_set(struct rte_eth_dev *dev, enum rte_vlan_type vlan_type, uint16_t tpid_id); -static void eth_igb_vlan_offload_set(struct rte_eth_dev *dev, int mask); +static int eth_igb_vlan_offload_set(struct rte_eth_dev *dev, int mask); static void igb_vlan_hw_filter_enable(struct rte_eth_dev *dev); static void igb_vlan_hw_filter_disable(struct rte_eth_dev *dev); @@ -1400,7 +1400,12 @@ static int eth_igbvf_pci_remove(struct rte_pci_device *pci_dev) */ mask = ETH_VLAN_STRIP_MASK | ETH_VLAN_FILTER_MASK | \ ETH_VLAN_EXTEND_MASK; - eth_igb_vlan_offload_set(dev, mask); + ret = eth_igb_vlan_offload_set(dev, mask); + if (ret) { + PMD_INIT_LOG(ERR, "Unable to set vlan offload"); + igb_dev_clear_queues(dev); + return ret; + } if (dev->data->dev_conf.rxmode.mq_mode == ETH_MQ_RX_VMDQ_ONLY) { /* Enable VLAN filter since VMDq always use VLAN filter */ @@ -2715,7 +2720,7 @@ static int eth_igbvf_xstats_get_names(__rte_unused struct rte_eth_dev *dev, 2 * VLAN_TAG_SIZE); } -static void +static int eth_igb_vlan_offload_set(struct rte_eth_dev *dev, int mask) { if(mask & ETH_VLAN_STRIP_MASK){ @@ -2738,6 +2743,7 @@ static int eth_igbvf_xstats_get_names(__rte_unused struct rte_eth_dev *dev, else igb_vlan_hw_extend_disable(dev); } + return 0; } diff --git a/drivers/net/enic/enic_ethdev.c b/drivers/net/enic/enic_ethdev.c index da8fec2..fc1eac2 100644 --- a/drivers/net/enic/enic_ethdev.c +++ b/drivers/net/enic/enic_ethdev.c @@ -347,7 +347,7 @@ static int enicpmd_vlan_filter_set(struct rte_eth_dev *eth_dev, return err; } -static void enicpmd_vlan_offload_set(struct rte_eth_dev *eth_dev, int mask) +static int enicpmd_vlan_offload_set(struct rte_eth_dev *eth_dev, int mask) { struct enic *enic = pmd_priv(eth_dev); @@ -371,6 +371,8 @@ static void enicpmd_vlan_offload_set(struct rte_eth_dev *eth_dev, int mask) dev_warning(enic, "Configuration of extended VLAN is not supported\n"); } + + return 0; } static int enicpmd_dev_configure(struct rte_eth_dev *eth_dev) @@ -392,9 +394,9 @@ static int enicpmd_dev_configure(struct rte_eth_dev *eth_dev) eth_dev->data->dev_conf.rxmode.split_hdr_size); } - enicpmd_vlan_offload_set(eth_dev, ETH_VLAN_STRIP_MASK); + ret = enicpmd_vlan_offload_set(eth_dev, ETH_VLAN_STRIP_MASK); enic->hw_ip_checksum = eth_dev->data->dev_conf.rxmode.hw_ip_checksum; - return 0; + return ret; } /* Start the device. diff --git a/drivers/net/fm10k/fm10k_ethdev.c b/drivers/net/fm10k/fm10k_ethdev.c index e60d3a3..f4626f7 100644 --- a/drivers/net/fm10k/fm10k_ethdev.c +++ b/drivers/net/fm10k/fm10k_ethdev.c @@ -1590,7 +1590,7 @@ static int fm10k_xstats_get_names(__rte_unused struct rte_eth_dev *dev, return 0; } -static void +static int fm10k_vlan_offload_set(struct rte_eth_dev *dev, int mask) { if (mask & ETH_VLAN_STRIP_MASK) { @@ -1609,6 +1609,7 @@ static int fm10k_xstats_get_names(__rte_unused struct rte_eth_dev *dev, if (!dev->data->dev_conf.rxmode.hw_vlan_filter) PMD_INIT_LOG(ERR, "VLAN filter is always on in fm10k"); } + return 0; } /* Add/Remove a MAC address, and update filters to main VSI */ diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c index 00b6082..d03a44b 100644 --- a/drivers/net/i40e/i40e_ethdev.c +++ b/drivers/net/i40e/i40e_ethdev.c @@ -278,7 +278,7 @@ static int i40e_vlan_filter_set(struct rte_eth_dev *dev, static int i40e_vlan_tpid_set(struct rte_eth_dev *dev, enum rte_vlan_type vlan_type, uint16_t tpid); -static void i40e_vlan_offload_set(struct rte_eth_dev *dev, int mask); +static int i40e_vlan_offload_set(struct rte_eth_dev *dev, int mask); static void i40e_vlan_strip_queue_set(struct rte_eth_dev *dev, uint16_t queue, int on); @@ -3130,7 +3130,7 @@ static int i40e_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev, return ret; } -static void +static int i40e_vlan_offload_set(struct rte_eth_dev *dev, int mask) { struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private); @@ -3163,6 +3163,7 @@ static int i40e_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev, else i40e_vsi_config_double_vlan(vsi, FALSE); } + return 0; } static void @@ -5216,7 +5217,11 @@ struct i40e_vsi * /* Apply vlan offload setting */ mask = ETH_VLAN_STRIP_MASK | ETH_VLAN_FILTER_MASK; - i40e_vlan_offload_set(dev, mask); + ret = i40e_vlan_offload_set(dev, mask); + if (ret) { + PMD_DRV_LOG(INFO, "Failed to update vlan offload"); + return ret; + } /* Apply double-vlan setting, not implemented yet */ diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c index f6d8293..f7fffc2 100644 --- a/drivers/net/i40e/i40e_ethdev_vf.c +++ b/drivers/net/i40e/i40e_ethdev_vf.c @@ -118,7 +118,7 @@ static int i40evf_dev_xstats_get_names(struct rte_eth_dev *dev, static void i40evf_dev_xstats_reset(struct rte_eth_dev *dev); static int i40evf_vlan_filter_set(struct rte_eth_dev *dev, uint16_t vlan_id, int on); -static void i40evf_vlan_offload_set(struct rte_eth_dev *dev, int mask); +static int i40evf_vlan_offload_set(struct rte_eth_dev *dev, int mask); static int i40evf_vlan_pvid_set(struct rte_eth_dev *dev, uint16_t pvid, int on); static void i40evf_dev_close(struct rte_eth_dev *dev); @@ -1634,7 +1634,9 @@ static int eth_i40evf_pci_remove(struct rte_pci_device *pci_dev) int ret; /* Apply vlan offload setting */ - i40evf_vlan_offload_set(dev, ETH_VLAN_STRIP_MASK); + ret = i40evf_vlan_offload_set(dev, ETH_VLAN_STRIP_MASK); + if (ret) + return ret; /* Apply pvid setting */ ret = i40evf_vlan_pvid_set(dev, data->dev_conf.txmode.pvid, @@ -1642,7 +1644,7 @@ static int eth_i40evf_pci_remove(struct rte_pci_device *pci_dev) return ret; } -static void +static int i40evf_vlan_offload_set(struct rte_eth_dev *dev, int mask) { struct rte_eth_conf *dev_conf = &dev->data->dev_conf; @@ -1655,6 +1657,7 @@ static int eth_i40evf_pci_remove(struct rte_pci_device *pci_dev) else i40evf_disable_vlan_strip(dev); } + return 0; } static int diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c index 22171d8..1ec5aaf 100644 --- a/drivers/net/ixgbe/ixgbe_ethdev.c +++ b/drivers/net/ixgbe/ixgbe_ethdev.c @@ -218,7 +218,7 @@ static void ixgbe_vlan_hw_strip_bitmap_set(struct rte_eth_dev *dev, uint16_t queue, bool on); static void ixgbe_vlan_strip_queue_set(struct rte_eth_dev *dev, uint16_t queue, int on); -static void ixgbe_vlan_offload_set(struct rte_eth_dev *dev, int mask); +static int ixgbe_vlan_offload_set(struct rte_eth_dev *dev, int mask); static void ixgbe_vlan_hw_strip_enable(struct rte_eth_dev *dev, uint16_t queue); static void ixgbe_vlan_hw_strip_disable(struct rte_eth_dev *dev, uint16_t queue); static void ixgbe_vlan_hw_extend_enable(struct rte_eth_dev *dev); @@ -274,7 +274,7 @@ static int ixgbevf_vlan_filter_set(struct rte_eth_dev *dev, uint16_t vlan_id, int on); static void ixgbevf_vlan_strip_queue_set(struct rte_eth_dev *dev, uint16_t queue, int on); -static void ixgbevf_vlan_offload_set(struct rte_eth_dev *dev, int mask); +static int ixgbevf_vlan_offload_set(struct rte_eth_dev *dev, int mask); static void ixgbevf_set_vfta_all(struct rte_eth_dev *dev, bool on); static int ixgbevf_dev_rx_queue_intr_enable(struct rte_eth_dev *dev, uint16_t queue_id); @@ -2125,7 +2125,7 @@ static int eth_ixgbevf_pci_remove(struct rte_pci_device *pci_dev) */ } -static void +static int ixgbe_vlan_offload_set(struct rte_eth_dev *dev, int mask) { if (mask & ETH_VLAN_STRIP_MASK) { @@ -2148,6 +2148,7 @@ static int eth_ixgbevf_pci_remove(struct rte_pci_device *pci_dev) else ixgbe_vlan_hw_extend_disable(dev); } + return 0; } static void @@ -2568,9 +2569,13 @@ static int eth_ixgbevf_pci_remove(struct rte_pci_device *pci_dev) goto error; } - mask = ETH_VLAN_STRIP_MASK | ETH_VLAN_FILTER_MASK | + mask = ETH_VLAN_STRIP_MASK | ETH_VLAN_FILTER_MASK | ETH_VLAN_EXTEND_MASK; - ixgbe_vlan_offload_set(dev, mask); + err = ixgbe_vlan_offload_set(dev, mask); + if (err) { + PMD_INIT_LOG(ERR, "Unable to set VLAN offload"); + goto error; + } if (dev->data->dev_conf.rxmode.mq_mode == ETH_MQ_RX_VMDQ_ONLY) { /* Enable vlan filtering for VMDq */ @@ -4993,7 +4998,12 @@ static int ixgbevf_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev, /* Set HW strip */ mask = ETH_VLAN_STRIP_MASK | ETH_VLAN_FILTER_MASK | ETH_VLAN_EXTEND_MASK; - ixgbevf_vlan_offload_set(dev, mask); + err = ixgbevf_vlan_offload_set(dev, mask); + if (err) { + PMD_INIT_LOG(ERR, "Unable to set VLAN offload (%d)", err); + ixgbe_dev_clear_queues(dev); + return err; + } ixgbevf_dev_rxtx_start(dev); @@ -5153,7 +5163,7 @@ static void ixgbevf_set_vfta_all(struct rte_eth_dev *dev, bool on) ixgbe_vlan_hw_strip_bitmap_set(dev, queue, on); } -static void +static int ixgbevf_vlan_offload_set(struct rte_eth_dev *dev, int mask) { struct ixgbe_hw *hw = @@ -5168,6 +5178,7 @@ static void ixgbevf_set_vfta_all(struct rte_eth_dev *dev, bool on) for (i = 0; i < hw->mac.max_rx_queues; i++) ixgbevf_vlan_strip_queue_set(dev, i, on); } + return 0; } int diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h index 43c5384..93e71be 100644 --- a/drivers/net/mlx5/mlx5.h +++ b/drivers/net/mlx5/mlx5.h @@ -283,7 +283,7 @@ int mlx5_xstats_get_names(struct rte_eth_dev *, /* mlx5_vlan.c */ int mlx5_vlan_filter_set(struct rte_eth_dev *, uint16_t, int); -void mlx5_vlan_offload_set(struct rte_eth_dev *, int); +int mlx5_vlan_offload_set(struct rte_eth_dev *, int); void mlx5_vlan_strip_queue_set(struct rte_eth_dev *, uint16_t, int); /* mlx5_trigger.c */ diff --git a/drivers/net/mlx5/mlx5_vlan.c b/drivers/net/mlx5/mlx5_vlan.c index 1b0fa40..7215909 100644 --- a/drivers/net/mlx5/mlx5_vlan.c +++ b/drivers/net/mlx5/mlx5_vlan.c @@ -210,7 +210,7 @@ * @param mask * VLAN offload bit mask. */ -void +int mlx5_vlan_offload_set(struct rte_eth_dev *dev, int mask) { struct priv *priv = dev->data->dev_private; @@ -230,4 +230,5 @@ priv_vlan_strip_queue_set(priv, i, hw_vlan_strip); priv_unlock(priv); } + return 0; } diff --git a/drivers/net/nfp/nfp_net.c b/drivers/net/nfp/nfp_net.c index 92b03c4..6473edc 100644 --- a/drivers/net/nfp/nfp_net.c +++ b/drivers/net/nfp/nfp_net.c @@ -2149,11 +2149,12 @@ uint32_t nfp_net_txq_full(struct nfp_net_txq *txq) return i; } -static void +static int nfp_net_vlan_offload_set(struct rte_eth_dev *dev, int mask) { uint32_t new_ctrl, update; struct nfp_net_hw *hw; + int ret; hw = NFP_NET_DEV_PRIVATE_TO_HW(dev->data->dev_private); new_ctrl = 0; @@ -2174,14 +2175,14 @@ uint32_t nfp_net_txq_full(struct nfp_net_txq *txq) new_ctrl = hw->ctrl & ~NFP_NET_CFG_CTRL_RXVLAN; if (new_ctrl == 0) - return; + return 0; update = NFP_NET_CFG_UPDATE_GEN; - if (nfp_net_reconfig(hw, new_ctrl, update) < 0) - return; - - hw->ctrl = new_ctrl; + ret = nfp_net_reconfig(hw, new_ctrl, update); + if (!ret) + hw->ctrl = new_ctrl; + return ret; } /* Update Redirection Table(RETA) of Receive Side Scaling of Ethernet device */ diff --git a/drivers/net/qede/qede_ethdev.c b/drivers/net/qede/qede_ethdev.c index 0e05989..c5b1653 100644 --- a/drivers/net/qede/qede_ethdev.c +++ b/drivers/net/qede/qede_ethdev.c @@ -975,7 +975,7 @@ static int qede_vlan_filter_set(struct rte_eth_dev *eth_dev, return rc; } -static void qede_vlan_offload_set(struct rte_eth_dev *eth_dev, int mask) +static int qede_vlan_offload_set(struct rte_eth_dev *eth_dev, int mask) { struct qede_dev *qdev = QEDE_INIT_QDEV(eth_dev); struct ecore_dev *edev = QEDE_INIT_EDEV(qdev); @@ -1013,6 +1013,8 @@ static void qede_vlan_offload_set(struct rte_eth_dev *eth_dev, int mask) DP_INFO(edev, "vlan offload mask %d vlan-strip %d vlan-filter %d\n", mask, rxmode->hw_vlan_strip, rxmode->hw_vlan_filter); + + return 0; } static void qede_prandom_bytes(uint32_t *buff) @@ -1157,6 +1159,7 @@ static int qede_dev_configure(struct rte_eth_dev *eth_dev) struct qede_dev *qdev = QEDE_INIT_QDEV(eth_dev); struct ecore_dev *edev = QEDE_INIT_EDEV(qdev); struct rte_eth_rxmode *rxmode = ð_dev->data->dev_conf.rxmode; + int ret; PMD_INIT_FUNC_TRACE(edev); @@ -1237,9 +1240,11 @@ static int qede_dev_configure(struct rte_eth_dev *eth_dev) qdev->enable_lro = rxmode->enable_lro; /* Enable VLAN offloads by default */ - qede_vlan_offload_set(eth_dev, ETH_VLAN_STRIP_MASK | + ret = qede_vlan_offload_set(eth_dev, ETH_VLAN_STRIP_MASK | ETH_VLAN_FILTER_MASK | ETH_VLAN_EXTEND_MASK); + if (ret) + return ret; DP_INFO(edev, "Device configured with RSS=%d TSS=%d\n", QEDE_RSS_COUNT(qdev), QEDE_TSS_COUNT(qdev)); diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c index e320811..72b4248 100644 --- a/drivers/net/virtio/virtio_ethdev.c +++ b/drivers/net/virtio/virtio_ethdev.c @@ -72,6 +72,7 @@ static void virtio_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info); static int virtio_dev_link_update(struct rte_eth_dev *dev, int wait_to_complete); +static int virtio_dev_vlan_offload_set(struct rte_eth_dev *dev, int mask); static void virtio_set_hwaddr(struct virtio_hw *hw); static void virtio_get_hwaddr(struct virtio_hw *hw); @@ -779,6 +780,7 @@ struct rte_virtio_xstats_name_off { .stats_reset = virtio_dev_stats_reset, .xstats_reset = virtio_dev_stats_reset, .link_update = virtio_dev_link_update, + .vlan_offload_set = virtio_dev_vlan_offload_set, .rx_queue_setup = virtio_dev_rx_queue_setup, .rx_queue_intr_enable = virtio_dev_rx_queue_intr_enable, .rx_queue_intr_disable = virtio_dev_rx_queue_intr_disable, @@ -1875,6 +1877,25 @@ static void virtio_dev_free_mbufs(struct rte_eth_dev *dev) return (old.link_status == link.link_status) ? -1 : 0; } +static int +virtio_dev_vlan_offload_set(struct rte_eth_dev *dev, int mask) +{ + const struct rte_eth_rxmode *rxmode = &dev->data->dev_conf.rxmode; + struct virtio_hw *hw = dev->data->dev_private; + + if (rxmode->hw_vlan_filter && + !vtpci_with_feature(hw, VIRTIO_NET_F_CTRL_VLAN)) { + PMD_DRV_LOG(NOTICE, + "vlan filtering not available on this host"); + return -ENOTSUP; + } + + if (mask & ETH_VLAN_STRIP_MASK) + hw->vlan_strip = rxmode->hw_vlan_strip; + + return 0; +} + static void virtio_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info) { diff --git a/drivers/net/vmxnet3/vmxnet3_ethdev.c b/drivers/net/vmxnet3/vmxnet3_ethdev.c index 3910991..06735dd 100644 --- a/drivers/net/vmxnet3/vmxnet3_ethdev.c +++ b/drivers/net/vmxnet3/vmxnet3_ethdev.c @@ -100,7 +100,7 @@ static void vmxnet3_dev_info_get(struct rte_eth_dev *dev, 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 void vmxnet3_dev_vlan_offload_set(struct rte_eth_dev *dev, int mask); +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, struct ether_addr *mac_addr); static void vmxnet3_interrupt_handler(void *param); @@ -730,8 +730,10 @@ static int eth_vmxnet3_pci_remove(struct rte_pci_device *pci_dev) devRead->rssConfDesc.confPA = hw->rss_confPA; } - vmxnet3_dev_vlan_offload_set(dev, + ret = vmxnet3_dev_vlan_offload_set(dev, ETH_VLAN_STRIP_MASK | ETH_VLAN_FILTER_MASK); + if (ret) + return ret; vmxnet3_write_mac(hw, dev->data->mac_addrs->addr_bytes); @@ -1275,7 +1277,7 @@ static int eth_vmxnet3_pci_remove(struct rte_pci_device *pci_dev) return 0; } -static void +static int vmxnet3_dev_vlan_offload_set(struct rte_eth_dev *dev, int mask) { struct vmxnet3_hw *hw = dev->data->dev_private; @@ -1301,6 +1303,8 @@ static int eth_vmxnet3_pci_remove(struct rte_pci_device *pci_dev) VMXNET3_WRITE_BAR1_REG(hw, VMXNET3_REG_CMD, VMXNET3_CMD_UPDATE_VLAN_FILTERS); } + + return 0; } static void diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c index 0597641..d592a96 100644 --- a/lib/librte_ether/rte_ethdev.c +++ b/lib/librte_ether/rte_ethdev.c @@ -2049,10 +2049,16 @@ struct rte_eth_dev * int ret = 0; int mask = 0; int cur, org = 0; + uint8_t org_strip, org_filter, org_extend; RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV); dev = &rte_eth_devices[port_id]; + /* save original values in case of failure */ + org_strip = dev->data->dev_conf.rxmode.hw_vlan_strip; + org_filter = dev->data->dev_conf.rxmode.hw_vlan_filter; + org_extend = dev->data->dev_conf.rxmode.hw_vlan_extend; + /*check which option changed by application*/ cur = !!(offload_mask & ETH_VLAN_STRIP_OFFLOAD); org = !!(dev->data->dev_conf.rxmode.hw_vlan_strip); @@ -2080,7 +2086,13 @@ struct rte_eth_dev * return ret; RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->vlan_offload_set, -ENOTSUP); - (*dev->dev_ops->vlan_offload_set)(dev, mask); + ret = (*dev->dev_ops->vlan_offload_set)(dev, mask); + if (ret) { + /* hit an error restore original values */ + dev->data->dev_conf.rxmode.hw_vlan_strip = org_strip; + dev->data->dev_conf.rxmode.hw_vlan_filter = org_filter; + dev->data->dev_conf.rxmode.hw_vlan_extend = org_extend; + } return ret; } diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h index 0adf327..7254fd0 100644 --- a/lib/librte_ether/rte_ethdev.h +++ b/lib/librte_ether/rte_ethdev.h @@ -1245,7 +1245,7 @@ typedef int (*vlan_tpid_set_t)(struct rte_eth_dev *dev, enum rte_vlan_type type, uint16_t tpid); /**< @internal set the outer/inner VLAN-TPID by an Ethernet device. */ -typedef void (*vlan_offload_set_t)(struct rte_eth_dev *dev, int mask); +typedef int (*vlan_offload_set_t)(struct rte_eth_dev *dev, int mask); /**< @internal set VLAN offload function by an Ethernet device. */ typedef int (*vlan_pvid_set_t)(struct rte_eth_dev *dev, -- 1.8.3.1 ^ permalink raw reply [flat|nested] 19+ messages in thread
* [dpdk-dev] [PATCH v3] ethdev: modifiy vlan_offload_set_t to return int 2017-08-25 13:33 ` [dpdk-dev] [PATCH v2] " David Harton @ 2017-08-25 13:47 ` David Harton 2017-08-31 22:04 ` Thomas Monjalon 2017-09-01 2:36 ` [dpdk-dev] [PATCH v4] ethdev: allow returning error on VLAN offload configuration David Harton 0 siblings, 2 replies; 19+ messages in thread From: David Harton @ 2017-08-25 13:47 UTC (permalink / raw) To: thomas, ferruh.yigit, ajit.khaparde, johndale, konstantin.ananyev, jingjing.wu, beilei.xing, jing.d.chen, adrien.mazarguil, nelio.laranjeiro, alejandro.lucero, hemant.agrawal, rasesh.mody, harish.patil, skhare, yliu, maxime.coquelin, allain.legacy Cc: dev, David Harton Some devices may not support or fail setting VLAN offload configuration based on dynamic circurmstances so the vlan_offload_set_t vector is modified to return an int so the caller can determine success or not. rte_eth_dev_set_vlan_offload is updated to return the value provided by the vector when called along with restoring the original offload configs on failure. Existing vlan_offload_set_t vectors are modified to return an int. Majority of cases return 0 but a few that actually can fail now return their failure codes. Finally, a vlan_offload_set_t vector is added to virtio to facilitate dynamically turning VLAN strip on or off. Signed-off-by: David Harton <dharton@cisco.com> --- *v3 Fixed a format error. Apologies...need to figure out why checkpatches.pl keeps saying valid patch when I've got soft tabs. *v2 Fixed a missed format error. Removed vlan offload vector call casts and replaced with checks for return values. *v1 This is an ABI breakage that has been previously negotiated with Thomas and the proposed rel note change is included as well. doc/guides/rel_notes/release_17_11.rst | 2 +- drivers/net/avp/avp_ethdev.c | 12 +++++++++--- drivers/net/bnxt/bnxt_ethdev.c | 9 ++++++--- drivers/net/dpaa2/dpaa2_ethdev.c | 13 ++++++++++--- drivers/net/e1000/em_ethdev.c | 12 +++++++++--- drivers/net/e1000/igb_ethdev.c | 12 +++++++++--- drivers/net/enic/enic_ethdev.c | 8 +++++--- drivers/net/fm10k/fm10k_ethdev.c | 3 ++- drivers/net/i40e/i40e_ethdev.c | 11 ++++++++--- drivers/net/i40e/i40e_ethdev_vf.c | 9 ++++++--- drivers/net/ixgbe/ixgbe_ethdev.c | 25 ++++++++++++++++++------- drivers/net/mlx5/mlx5.h | 2 +- drivers/net/mlx5/mlx5_vlan.c | 3 ++- drivers/net/nfp/nfp_net.c | 13 +++++++------ drivers/net/qede/qede_ethdev.c | 9 +++++++-- drivers/net/virtio/virtio_ethdev.c | 21 +++++++++++++++++++++ drivers/net/vmxnet3/vmxnet3_ethdev.c | 10 +++++++--- lib/librte_ether/rte_ethdev.c | 14 +++++++++++++- lib/librte_ether/rte_ethdev.h | 2 +- 19 files changed, 142 insertions(+), 48 deletions(-) diff --git a/doc/guides/rel_notes/release_17_11.rst b/doc/guides/rel_notes/release_17_11.rst index 170f4f9..e74f70c 100644 --- a/doc/guides/rel_notes/release_17_11.rst +++ b/doc/guides/rel_notes/release_17_11.rst @@ -124,7 +124,7 @@ ABI Changes Also, make sure to start the actual text at the margin. ========================================================= - +* Changed return type of ``vlan_offload_set_t`` from ``void`` to ``int``. Shared Library Versions ----------------------- diff --git a/drivers/net/avp/avp_ethdev.c b/drivers/net/avp/avp_ethdev.c index c746a0e..4011dfa 100644 --- a/drivers/net/avp/avp_ethdev.c +++ b/drivers/net/avp/avp_ethdev.c @@ -70,7 +70,7 @@ static int avp_dev_create(struct rte_pci_device *pci_dev, static void avp_dev_close(struct rte_eth_dev *dev); static void avp_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info); -static void avp_vlan_offload_set(struct rte_eth_dev *dev, int mask); +static int avp_vlan_offload_set(struct rte_eth_dev *dev, int mask); static int avp_dev_link_update(struct rte_eth_dev *dev, int wait_to_complete); static void avp_dev_promiscuous_enable(struct rte_eth_dev *dev); static void avp_dev_promiscuous_disable(struct rte_eth_dev *dev); @@ -2031,7 +2031,12 @@ struct avp_queue { mask = (ETH_VLAN_STRIP_MASK | ETH_VLAN_FILTER_MASK | ETH_VLAN_EXTEND_MASK); - avp_vlan_offload_set(eth_dev, mask); + ret = avp_vlan_offload_set(eth_dev, mask); + if (ret < 0) { + PMD_DRV_LOG(ERR, "VLAN offload set failed by host, ret=%d\n", + ret); + goto unlock; + } /* update device config */ memset(&config, 0, sizeof(config)); @@ -2214,7 +2219,7 @@ struct avp_queue { } } -static void +static int avp_vlan_offload_set(struct rte_eth_dev *eth_dev, int mask) { struct avp_dev *avp = AVP_DEV_PRIVATE_TO_HW(eth_dev->data->dev_private); @@ -2239,6 +2244,7 @@ struct avp_queue { if (eth_dev->data->dev_conf.rxmode.hw_vlan_extend) PMD_DRV_LOG(ERR, "VLAN extend offload not supported\n"); } + return 0; } static void diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c index c9d1122..547bd55 100644 --- a/drivers/net/bnxt/bnxt_ethdev.c +++ b/drivers/net/bnxt/bnxt_ethdev.c @@ -144,7 +144,7 @@ ETH_RSS_NONFRAG_IPV6_TCP | \ ETH_RSS_NONFRAG_IPV6_UDP) -static void bnxt_vlan_offload_set_op(struct rte_eth_dev *dev, int mask); +static int bnxt_vlan_offload_set_op(struct rte_eth_dev *dev, int mask); /***********************/ @@ -522,7 +522,9 @@ static int bnxt_dev_start_op(struct rte_eth_dev *eth_dev) vlan_mask |= ETH_VLAN_FILTER_MASK; if (eth_dev->data->dev_conf.rxmode.hw_vlan_strip) vlan_mask |= ETH_VLAN_STRIP_MASK; - bnxt_vlan_offload_set_op(eth_dev, vlan_mask); + rc = bnxt_vlan_offload_set_op(eth_dev, vlan_mask); + if (rc) + goto error; return 0; @@ -1275,7 +1277,7 @@ static int bnxt_vlan_filter_set_op(struct rte_eth_dev *eth_dev, return bnxt_del_vlan_filter(bp, vlan_id); } -static void +static int bnxt_vlan_offload_set_op(struct rte_eth_dev *dev, int mask) { struct bnxt *bp = (struct bnxt *)dev->data->dev_private; @@ -1307,6 +1309,7 @@ static int bnxt_vlan_filter_set_op(struct rte_eth_dev *eth_dev, if (mask & ETH_VLAN_EXTEND_MASK) RTE_LOG(ERR, PMD, "Extend VLAN Not supported\n"); + return 0; } static void diff --git a/drivers/net/dpaa2/dpaa2_ethdev.c b/drivers/net/dpaa2/dpaa2_ethdev.c index 429b3a0..3390cb3 100644 --- a/drivers/net/dpaa2/dpaa2_ethdev.c +++ b/drivers/net/dpaa2/dpaa2_ethdev.c @@ -138,7 +138,7 @@ return ret; } -static void +static int dpaa2_vlan_offload_set(struct rte_eth_dev *dev, int mask) { struct dpaa2_dev_priv *priv = dev->data->dev_private; @@ -158,6 +158,7 @@ RTE_LOG(ERR, PMD, "Unable to set vlan filter = %d\n", ret); } + return 0; } static int @@ -643,8 +644,14 @@ return ret; } /* VLAN Offload Settings */ - if (priv->max_vlan_filters) - dpaa2_vlan_offload_set(dev, ETH_VLAN_FILTER_MASK); + if (priv->max_vlan_filters) { + ret = dpaa2_vlan_offload_set(dev, ETH_VLAN_FILTER_MASK); + if (ret) { + PMD_INIT_LOG(ERR, "Error to dpaa2_vlan_offload_set:" + "code = %d\n", ret); + return ret; + } + } return 0; } diff --git a/drivers/net/e1000/em_ethdev.c b/drivers/net/e1000/em_ethdev.c index 3d4ab93..51f49d8 100644 --- a/drivers/net/e1000/em_ethdev.c +++ b/drivers/net/e1000/em_ethdev.c @@ -99,7 +99,7 @@ static int eth_em_interrupt_action(struct rte_eth_dev *dev, static int eth_em_vlan_filter_set(struct rte_eth_dev *dev, uint16_t vlan_id, int on); -static void eth_em_vlan_offload_set(struct rte_eth_dev *dev, int mask); +static int eth_em_vlan_offload_set(struct rte_eth_dev *dev, int mask); static void em_vlan_hw_filter_enable(struct rte_eth_dev *dev); static void em_vlan_hw_filter_disable(struct rte_eth_dev *dev); static void em_vlan_hw_strip_enable(struct rte_eth_dev *dev); @@ -668,7 +668,12 @@ static int eth_em_pci_remove(struct rte_pci_device *pci_dev) mask = ETH_VLAN_STRIP_MASK | ETH_VLAN_FILTER_MASK | \ ETH_VLAN_EXTEND_MASK; - eth_em_vlan_offload_set(dev, mask); + ret = eth_em_vlan_offload_set(dev, mask); + if (ret) { + PMD_INIT_LOG(ERR, "Unable to update vlan offload"); + em_dev_clear_queues(dev); + return ret; + } /* Set Interrupt Throttling Rate to maximum allowed value. */ E1000_WRITE_REG(hw, E1000_ITR, UINT16_MAX); @@ -1447,7 +1452,7 @@ static int eth_em_pci_remove(struct rte_pci_device *pci_dev) E1000_WRITE_REG(hw, E1000_CTRL, reg); } -static void +static int eth_em_vlan_offload_set(struct rte_eth_dev *dev, int mask) { if(mask & ETH_VLAN_STRIP_MASK){ @@ -1463,6 +1468,7 @@ static int eth_em_pci_remove(struct rte_pci_device *pci_dev) else em_vlan_hw_filter_disable(dev); } + return 0; } /* diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c index e4f7a9f..fa15ee9 100644 --- a/drivers/net/e1000/igb_ethdev.c +++ b/drivers/net/e1000/igb_ethdev.c @@ -157,7 +157,7 @@ static int eth_igb_vlan_filter_set(struct rte_eth_dev *dev, static int eth_igb_vlan_tpid_set(struct rte_eth_dev *dev, enum rte_vlan_type vlan_type, uint16_t tpid_id); -static void eth_igb_vlan_offload_set(struct rte_eth_dev *dev, int mask); +static int eth_igb_vlan_offload_set(struct rte_eth_dev *dev, int mask); static void igb_vlan_hw_filter_enable(struct rte_eth_dev *dev); static void igb_vlan_hw_filter_disable(struct rte_eth_dev *dev); @@ -1400,7 +1400,12 @@ static int eth_igbvf_pci_remove(struct rte_pci_device *pci_dev) */ mask = ETH_VLAN_STRIP_MASK | ETH_VLAN_FILTER_MASK | \ ETH_VLAN_EXTEND_MASK; - eth_igb_vlan_offload_set(dev, mask); + ret = eth_igb_vlan_offload_set(dev, mask); + if (ret) { + PMD_INIT_LOG(ERR, "Unable to set vlan offload"); + igb_dev_clear_queues(dev); + return ret; + } if (dev->data->dev_conf.rxmode.mq_mode == ETH_MQ_RX_VMDQ_ONLY) { /* Enable VLAN filter since VMDq always use VLAN filter */ @@ -2715,7 +2720,7 @@ static int eth_igbvf_xstats_get_names(__rte_unused struct rte_eth_dev *dev, 2 * VLAN_TAG_SIZE); } -static void +static int eth_igb_vlan_offload_set(struct rte_eth_dev *dev, int mask) { if(mask & ETH_VLAN_STRIP_MASK){ @@ -2738,6 +2743,7 @@ static int eth_igbvf_xstats_get_names(__rte_unused struct rte_eth_dev *dev, else igb_vlan_hw_extend_disable(dev); } + return 0; } diff --git a/drivers/net/enic/enic_ethdev.c b/drivers/net/enic/enic_ethdev.c index da8fec2..fc1eac2 100644 --- a/drivers/net/enic/enic_ethdev.c +++ b/drivers/net/enic/enic_ethdev.c @@ -347,7 +347,7 @@ static int enicpmd_vlan_filter_set(struct rte_eth_dev *eth_dev, return err; } -static void enicpmd_vlan_offload_set(struct rte_eth_dev *eth_dev, int mask) +static int enicpmd_vlan_offload_set(struct rte_eth_dev *eth_dev, int mask) { struct enic *enic = pmd_priv(eth_dev); @@ -371,6 +371,8 @@ static void enicpmd_vlan_offload_set(struct rte_eth_dev *eth_dev, int mask) dev_warning(enic, "Configuration of extended VLAN is not supported\n"); } + + return 0; } static int enicpmd_dev_configure(struct rte_eth_dev *eth_dev) @@ -392,9 +394,9 @@ static int enicpmd_dev_configure(struct rte_eth_dev *eth_dev) eth_dev->data->dev_conf.rxmode.split_hdr_size); } - enicpmd_vlan_offload_set(eth_dev, ETH_VLAN_STRIP_MASK); + ret = enicpmd_vlan_offload_set(eth_dev, ETH_VLAN_STRIP_MASK); enic->hw_ip_checksum = eth_dev->data->dev_conf.rxmode.hw_ip_checksum; - return 0; + return ret; } /* Start the device. diff --git a/drivers/net/fm10k/fm10k_ethdev.c b/drivers/net/fm10k/fm10k_ethdev.c index e60d3a3..f4626f7 100644 --- a/drivers/net/fm10k/fm10k_ethdev.c +++ b/drivers/net/fm10k/fm10k_ethdev.c @@ -1590,7 +1590,7 @@ static int fm10k_xstats_get_names(__rte_unused struct rte_eth_dev *dev, return 0; } -static void +static int fm10k_vlan_offload_set(struct rte_eth_dev *dev, int mask) { if (mask & ETH_VLAN_STRIP_MASK) { @@ -1609,6 +1609,7 @@ static int fm10k_xstats_get_names(__rte_unused struct rte_eth_dev *dev, if (!dev->data->dev_conf.rxmode.hw_vlan_filter) PMD_INIT_LOG(ERR, "VLAN filter is always on in fm10k"); } + return 0; } /* Add/Remove a MAC address, and update filters to main VSI */ diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c index 00b6082..d03a44b 100644 --- a/drivers/net/i40e/i40e_ethdev.c +++ b/drivers/net/i40e/i40e_ethdev.c @@ -278,7 +278,7 @@ static int i40e_vlan_filter_set(struct rte_eth_dev *dev, static int i40e_vlan_tpid_set(struct rte_eth_dev *dev, enum rte_vlan_type vlan_type, uint16_t tpid); -static void i40e_vlan_offload_set(struct rte_eth_dev *dev, int mask); +static int i40e_vlan_offload_set(struct rte_eth_dev *dev, int mask); static void i40e_vlan_strip_queue_set(struct rte_eth_dev *dev, uint16_t queue, int on); @@ -3130,7 +3130,7 @@ static int i40e_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev, return ret; } -static void +static int i40e_vlan_offload_set(struct rte_eth_dev *dev, int mask) { struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private); @@ -3163,6 +3163,7 @@ static int i40e_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev, else i40e_vsi_config_double_vlan(vsi, FALSE); } + return 0; } static void @@ -5216,7 +5217,11 @@ struct i40e_vsi * /* Apply vlan offload setting */ mask = ETH_VLAN_STRIP_MASK | ETH_VLAN_FILTER_MASK; - i40e_vlan_offload_set(dev, mask); + ret = i40e_vlan_offload_set(dev, mask); + if (ret) { + PMD_DRV_LOG(INFO, "Failed to update vlan offload"); + return ret; + } /* Apply double-vlan setting, not implemented yet */ diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c index f6d8293..f7fffc2 100644 --- a/drivers/net/i40e/i40e_ethdev_vf.c +++ b/drivers/net/i40e/i40e_ethdev_vf.c @@ -118,7 +118,7 @@ static int i40evf_dev_xstats_get_names(struct rte_eth_dev *dev, static void i40evf_dev_xstats_reset(struct rte_eth_dev *dev); static int i40evf_vlan_filter_set(struct rte_eth_dev *dev, uint16_t vlan_id, int on); -static void i40evf_vlan_offload_set(struct rte_eth_dev *dev, int mask); +static int i40evf_vlan_offload_set(struct rte_eth_dev *dev, int mask); static int i40evf_vlan_pvid_set(struct rte_eth_dev *dev, uint16_t pvid, int on); static void i40evf_dev_close(struct rte_eth_dev *dev); @@ -1634,7 +1634,9 @@ static int eth_i40evf_pci_remove(struct rte_pci_device *pci_dev) int ret; /* Apply vlan offload setting */ - i40evf_vlan_offload_set(dev, ETH_VLAN_STRIP_MASK); + ret = i40evf_vlan_offload_set(dev, ETH_VLAN_STRIP_MASK); + if (ret) + return ret; /* Apply pvid setting */ ret = i40evf_vlan_pvid_set(dev, data->dev_conf.txmode.pvid, @@ -1642,7 +1644,7 @@ static int eth_i40evf_pci_remove(struct rte_pci_device *pci_dev) return ret; } -static void +static int i40evf_vlan_offload_set(struct rte_eth_dev *dev, int mask) { struct rte_eth_conf *dev_conf = &dev->data->dev_conf; @@ -1655,6 +1657,7 @@ static int eth_i40evf_pci_remove(struct rte_pci_device *pci_dev) else i40evf_disable_vlan_strip(dev); } + return 0; } static int diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c index 22171d8..1ec5aaf 100644 --- a/drivers/net/ixgbe/ixgbe_ethdev.c +++ b/drivers/net/ixgbe/ixgbe_ethdev.c @@ -218,7 +218,7 @@ static void ixgbe_vlan_hw_strip_bitmap_set(struct rte_eth_dev *dev, uint16_t queue, bool on); static void ixgbe_vlan_strip_queue_set(struct rte_eth_dev *dev, uint16_t queue, int on); -static void ixgbe_vlan_offload_set(struct rte_eth_dev *dev, int mask); +static int ixgbe_vlan_offload_set(struct rte_eth_dev *dev, int mask); static void ixgbe_vlan_hw_strip_enable(struct rte_eth_dev *dev, uint16_t queue); static void ixgbe_vlan_hw_strip_disable(struct rte_eth_dev *dev, uint16_t queue); static void ixgbe_vlan_hw_extend_enable(struct rte_eth_dev *dev); @@ -274,7 +274,7 @@ static int ixgbevf_vlan_filter_set(struct rte_eth_dev *dev, uint16_t vlan_id, int on); static void ixgbevf_vlan_strip_queue_set(struct rte_eth_dev *dev, uint16_t queue, int on); -static void ixgbevf_vlan_offload_set(struct rte_eth_dev *dev, int mask); +static int ixgbevf_vlan_offload_set(struct rte_eth_dev *dev, int mask); static void ixgbevf_set_vfta_all(struct rte_eth_dev *dev, bool on); static int ixgbevf_dev_rx_queue_intr_enable(struct rte_eth_dev *dev, uint16_t queue_id); @@ -2125,7 +2125,7 @@ static int eth_ixgbevf_pci_remove(struct rte_pci_device *pci_dev) */ } -static void +static int ixgbe_vlan_offload_set(struct rte_eth_dev *dev, int mask) { if (mask & ETH_VLAN_STRIP_MASK) { @@ -2148,6 +2148,7 @@ static int eth_ixgbevf_pci_remove(struct rte_pci_device *pci_dev) else ixgbe_vlan_hw_extend_disable(dev); } + return 0; } static void @@ -2568,9 +2569,13 @@ static int eth_ixgbevf_pci_remove(struct rte_pci_device *pci_dev) goto error; } - mask = ETH_VLAN_STRIP_MASK | ETH_VLAN_FILTER_MASK | + mask = ETH_VLAN_STRIP_MASK | ETH_VLAN_FILTER_MASK | ETH_VLAN_EXTEND_MASK; - ixgbe_vlan_offload_set(dev, mask); + err = ixgbe_vlan_offload_set(dev, mask); + if (err) { + PMD_INIT_LOG(ERR, "Unable to set VLAN offload"); + goto error; + } if (dev->data->dev_conf.rxmode.mq_mode == ETH_MQ_RX_VMDQ_ONLY) { /* Enable vlan filtering for VMDq */ @@ -4993,7 +4998,12 @@ static int ixgbevf_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev, /* Set HW strip */ mask = ETH_VLAN_STRIP_MASK | ETH_VLAN_FILTER_MASK | ETH_VLAN_EXTEND_MASK; - ixgbevf_vlan_offload_set(dev, mask); + err = ixgbevf_vlan_offload_set(dev, mask); + if (err) { + PMD_INIT_LOG(ERR, "Unable to set VLAN offload (%d)", err); + ixgbe_dev_clear_queues(dev); + return err; + } ixgbevf_dev_rxtx_start(dev); @@ -5153,7 +5163,7 @@ static void ixgbevf_set_vfta_all(struct rte_eth_dev *dev, bool on) ixgbe_vlan_hw_strip_bitmap_set(dev, queue, on); } -static void +static int ixgbevf_vlan_offload_set(struct rte_eth_dev *dev, int mask) { struct ixgbe_hw *hw = @@ -5168,6 +5178,7 @@ static void ixgbevf_set_vfta_all(struct rte_eth_dev *dev, bool on) for (i = 0; i < hw->mac.max_rx_queues; i++) ixgbevf_vlan_strip_queue_set(dev, i, on); } + return 0; } int diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h index 43c5384..93e71be 100644 --- a/drivers/net/mlx5/mlx5.h +++ b/drivers/net/mlx5/mlx5.h @@ -283,7 +283,7 @@ int mlx5_xstats_get_names(struct rte_eth_dev *, /* mlx5_vlan.c */ int mlx5_vlan_filter_set(struct rte_eth_dev *, uint16_t, int); -void mlx5_vlan_offload_set(struct rte_eth_dev *, int); +int mlx5_vlan_offload_set(struct rte_eth_dev *, int); void mlx5_vlan_strip_queue_set(struct rte_eth_dev *, uint16_t, int); /* mlx5_trigger.c */ diff --git a/drivers/net/mlx5/mlx5_vlan.c b/drivers/net/mlx5/mlx5_vlan.c index 1b0fa40..7215909 100644 --- a/drivers/net/mlx5/mlx5_vlan.c +++ b/drivers/net/mlx5/mlx5_vlan.c @@ -210,7 +210,7 @@ * @param mask * VLAN offload bit mask. */ -void +int mlx5_vlan_offload_set(struct rte_eth_dev *dev, int mask) { struct priv *priv = dev->data->dev_private; @@ -230,4 +230,5 @@ priv_vlan_strip_queue_set(priv, i, hw_vlan_strip); priv_unlock(priv); } + return 0; } diff --git a/drivers/net/nfp/nfp_net.c b/drivers/net/nfp/nfp_net.c index 92b03c4..6473edc 100644 --- a/drivers/net/nfp/nfp_net.c +++ b/drivers/net/nfp/nfp_net.c @@ -2149,11 +2149,12 @@ uint32_t nfp_net_txq_full(struct nfp_net_txq *txq) return i; } -static void +static int nfp_net_vlan_offload_set(struct rte_eth_dev *dev, int mask) { uint32_t new_ctrl, update; struct nfp_net_hw *hw; + int ret; hw = NFP_NET_DEV_PRIVATE_TO_HW(dev->data->dev_private); new_ctrl = 0; @@ -2174,14 +2175,14 @@ uint32_t nfp_net_txq_full(struct nfp_net_txq *txq) new_ctrl = hw->ctrl & ~NFP_NET_CFG_CTRL_RXVLAN; if (new_ctrl == 0) - return; + return 0; update = NFP_NET_CFG_UPDATE_GEN; - if (nfp_net_reconfig(hw, new_ctrl, update) < 0) - return; - - hw->ctrl = new_ctrl; + ret = nfp_net_reconfig(hw, new_ctrl, update); + if (!ret) + hw->ctrl = new_ctrl; + return ret; } /* Update Redirection Table(RETA) of Receive Side Scaling of Ethernet device */ diff --git a/drivers/net/qede/qede_ethdev.c b/drivers/net/qede/qede_ethdev.c index 0e05989..644f69d 100644 --- a/drivers/net/qede/qede_ethdev.c +++ b/drivers/net/qede/qede_ethdev.c @@ -975,7 +975,7 @@ static int qede_vlan_filter_set(struct rte_eth_dev *eth_dev, return rc; } -static void qede_vlan_offload_set(struct rte_eth_dev *eth_dev, int mask) +static int qede_vlan_offload_set(struct rte_eth_dev *eth_dev, int mask) { struct qede_dev *qdev = QEDE_INIT_QDEV(eth_dev); struct ecore_dev *edev = QEDE_INIT_EDEV(qdev); @@ -1013,6 +1013,8 @@ static void qede_vlan_offload_set(struct rte_eth_dev *eth_dev, int mask) DP_INFO(edev, "vlan offload mask %d vlan-strip %d vlan-filter %d\n", mask, rxmode->hw_vlan_strip, rxmode->hw_vlan_filter); + + return 0; } static void qede_prandom_bytes(uint32_t *buff) @@ -1157,6 +1159,7 @@ static int qede_dev_configure(struct rte_eth_dev *eth_dev) struct qede_dev *qdev = QEDE_INIT_QDEV(eth_dev); struct ecore_dev *edev = QEDE_INIT_EDEV(qdev); struct rte_eth_rxmode *rxmode = ð_dev->data->dev_conf.rxmode; + int ret; PMD_INIT_FUNC_TRACE(edev); @@ -1237,9 +1240,11 @@ static int qede_dev_configure(struct rte_eth_dev *eth_dev) qdev->enable_lro = rxmode->enable_lro; /* Enable VLAN offloads by default */ - qede_vlan_offload_set(eth_dev, ETH_VLAN_STRIP_MASK | + ret = qede_vlan_offload_set(eth_dev, ETH_VLAN_STRIP_MASK | ETH_VLAN_FILTER_MASK | ETH_VLAN_EXTEND_MASK); + if (ret) + return ret; DP_INFO(edev, "Device configured with RSS=%d TSS=%d\n", QEDE_RSS_COUNT(qdev), QEDE_TSS_COUNT(qdev)); diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c index e320811..72b4248 100644 --- a/drivers/net/virtio/virtio_ethdev.c +++ b/drivers/net/virtio/virtio_ethdev.c @@ -72,6 +72,7 @@ static void virtio_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info); static int virtio_dev_link_update(struct rte_eth_dev *dev, int wait_to_complete); +static int virtio_dev_vlan_offload_set(struct rte_eth_dev *dev, int mask); static void virtio_set_hwaddr(struct virtio_hw *hw); static void virtio_get_hwaddr(struct virtio_hw *hw); @@ -779,6 +780,7 @@ struct rte_virtio_xstats_name_off { .stats_reset = virtio_dev_stats_reset, .xstats_reset = virtio_dev_stats_reset, .link_update = virtio_dev_link_update, + .vlan_offload_set = virtio_dev_vlan_offload_set, .rx_queue_setup = virtio_dev_rx_queue_setup, .rx_queue_intr_enable = virtio_dev_rx_queue_intr_enable, .rx_queue_intr_disable = virtio_dev_rx_queue_intr_disable, @@ -1875,6 +1877,25 @@ static void virtio_dev_free_mbufs(struct rte_eth_dev *dev) return (old.link_status == link.link_status) ? -1 : 0; } +static int +virtio_dev_vlan_offload_set(struct rte_eth_dev *dev, int mask) +{ + const struct rte_eth_rxmode *rxmode = &dev->data->dev_conf.rxmode; + struct virtio_hw *hw = dev->data->dev_private; + + if (rxmode->hw_vlan_filter && + !vtpci_with_feature(hw, VIRTIO_NET_F_CTRL_VLAN)) { + PMD_DRV_LOG(NOTICE, + "vlan filtering not available on this host"); + return -ENOTSUP; + } + + if (mask & ETH_VLAN_STRIP_MASK) + hw->vlan_strip = rxmode->hw_vlan_strip; + + return 0; +} + static void virtio_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info) { diff --git a/drivers/net/vmxnet3/vmxnet3_ethdev.c b/drivers/net/vmxnet3/vmxnet3_ethdev.c index 3910991..06735dd 100644 --- a/drivers/net/vmxnet3/vmxnet3_ethdev.c +++ b/drivers/net/vmxnet3/vmxnet3_ethdev.c @@ -100,7 +100,7 @@ static void vmxnet3_dev_info_get(struct rte_eth_dev *dev, 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 void vmxnet3_dev_vlan_offload_set(struct rte_eth_dev *dev, int mask); +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, struct ether_addr *mac_addr); static void vmxnet3_interrupt_handler(void *param); @@ -730,8 +730,10 @@ static int eth_vmxnet3_pci_remove(struct rte_pci_device *pci_dev) devRead->rssConfDesc.confPA = hw->rss_confPA; } - vmxnet3_dev_vlan_offload_set(dev, + ret = vmxnet3_dev_vlan_offload_set(dev, ETH_VLAN_STRIP_MASK | ETH_VLAN_FILTER_MASK); + if (ret) + return ret; vmxnet3_write_mac(hw, dev->data->mac_addrs->addr_bytes); @@ -1275,7 +1277,7 @@ static int eth_vmxnet3_pci_remove(struct rte_pci_device *pci_dev) return 0; } -static void +static int vmxnet3_dev_vlan_offload_set(struct rte_eth_dev *dev, int mask) { struct vmxnet3_hw *hw = dev->data->dev_private; @@ -1301,6 +1303,8 @@ static int eth_vmxnet3_pci_remove(struct rte_pci_device *pci_dev) VMXNET3_WRITE_BAR1_REG(hw, VMXNET3_REG_CMD, VMXNET3_CMD_UPDATE_VLAN_FILTERS); } + + return 0; } static void diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c index 0597641..d592a96 100644 --- a/lib/librte_ether/rte_ethdev.c +++ b/lib/librte_ether/rte_ethdev.c @@ -2049,10 +2049,16 @@ struct rte_eth_dev * int ret = 0; int mask = 0; int cur, org = 0; + uint8_t org_strip, org_filter, org_extend; RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV); dev = &rte_eth_devices[port_id]; + /* save original values in case of failure */ + org_strip = dev->data->dev_conf.rxmode.hw_vlan_strip; + org_filter = dev->data->dev_conf.rxmode.hw_vlan_filter; + org_extend = dev->data->dev_conf.rxmode.hw_vlan_extend; + /*check which option changed by application*/ cur = !!(offload_mask & ETH_VLAN_STRIP_OFFLOAD); org = !!(dev->data->dev_conf.rxmode.hw_vlan_strip); @@ -2080,7 +2086,13 @@ struct rte_eth_dev * return ret; RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->vlan_offload_set, -ENOTSUP); - (*dev->dev_ops->vlan_offload_set)(dev, mask); + ret = (*dev->dev_ops->vlan_offload_set)(dev, mask); + if (ret) { + /* hit an error restore original values */ + dev->data->dev_conf.rxmode.hw_vlan_strip = org_strip; + dev->data->dev_conf.rxmode.hw_vlan_filter = org_filter; + dev->data->dev_conf.rxmode.hw_vlan_extend = org_extend; + } return ret; } diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h index 0adf327..7254fd0 100644 --- a/lib/librte_ether/rte_ethdev.h +++ b/lib/librte_ether/rte_ethdev.h @@ -1245,7 +1245,7 @@ typedef int (*vlan_tpid_set_t)(struct rte_eth_dev *dev, enum rte_vlan_type type, uint16_t tpid); /**< @internal set the outer/inner VLAN-TPID by an Ethernet device. */ -typedef void (*vlan_offload_set_t)(struct rte_eth_dev *dev, int mask); +typedef int (*vlan_offload_set_t)(struct rte_eth_dev *dev, int mask); /**< @internal set VLAN offload function by an Ethernet device. */ typedef int (*vlan_pvid_set_t)(struct rte_eth_dev *dev, -- 1.8.3.1 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [dpdk-dev] [PATCH v3] ethdev: modifiy vlan_offload_set_t to return int 2017-08-25 13:47 ` [dpdk-dev] [PATCH v3] " David Harton @ 2017-08-31 22:04 ` Thomas Monjalon 2017-08-31 22:08 ` Thomas Monjalon 2017-09-01 0:40 ` David Harton (dharton) 2017-09-01 2:36 ` [dpdk-dev] [PATCH v4] ethdev: allow returning error on VLAN offload configuration David Harton 1 sibling, 2 replies; 19+ messages in thread From: Thomas Monjalon @ 2017-08-31 22:04 UTC (permalink / raw) To: David Harton Cc: dev, ferruh.yigit, ajit.khaparde, johndale, konstantin.ananyev, jingjing.wu, beilei.xing, jing.d.chen, adrien.mazarguil, nelio.laranjeiro, alejandro.lucero, hemant.agrawal, rasesh.mody, harish.patil, skhare, yliu, maxime.coquelin, allain.legacy Hi, 25/08/2017 15:47, David Harton: > Some devices may not support or fail setting VLAN offload > configuration based on dynamic circurmstances so the > vlan_offload_set_t vector is modified to return an int so > the caller can determine success or not. I agree with allowing to return an error. Comments on details below. The title could be changed to better reflect the purpose: ethdev: allow returning error on VLAN configuration > --- a/doc/guides/rel_notes/release_17_11.rst > +++ b/doc/guides/rel_notes/release_17_11.rst > @@ -124,7 +124,7 @@ ABI Changes > Also, make sure to start the actual text at the margin. > ========================================================= > > - > +* Changed return type of ``vlan_offload_set_t`` from ``void`` to ``int``. It should be referenced as an API change (instead of ABI change). (and line spacing must be kept) We also need to bump the library version but it can be done with bigger changes in ethdev. > --- a/lib/librte_ether/rte_ethdev.c > +++ b/lib/librte_ether/rte_ethdev.c > @@ -2049,10 +2049,16 @@ struct rte_eth_dev * > int ret = 0; > int mask = 0; > int cur, org = 0; > + uint8_t org_strip, org_filter, org_extend; > > RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV); > dev = &rte_eth_devices[port_id]; > > + /* save original values in case of failure */ > + org_strip = dev->data->dev_conf.rxmode.hw_vlan_strip; > + org_filter = dev->data->dev_conf.rxmode.hw_vlan_filter; > + org_extend = dev->data->dev_conf.rxmode.hw_vlan_extend; Please waste one more char to write "orig" instead of "org". > - (*dev->dev_ops->vlan_offload_set)(dev, mask); > + ret = (*dev->dev_ops->vlan_offload_set)(dev, mask); > + if (ret) { > + /* hit an error restore original values */ > + dev->data->dev_conf.rxmode.hw_vlan_strip = org_strip; > + dev->data->dev_conf.rxmode.hw_vlan_filter = org_filter; > + dev->data->dev_conf.rxmode.hw_vlan_extend = org_extend; > + } Isn't it the responsibility of the PMD to restore values in case of error? I understand it is there to factorize error handling code, right? Do we want to document this behaviour with the ops prototype? ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [dpdk-dev] [PATCH v3] ethdev: modifiy vlan_offload_set_t to return int 2017-08-31 22:04 ` Thomas Monjalon @ 2017-08-31 22:08 ` Thomas Monjalon 2017-09-01 0:40 ` David Harton (dharton) 1 sibling, 0 replies; 19+ messages in thread From: Thomas Monjalon @ 2017-08-31 22:08 UTC (permalink / raw) To: David Harton Cc: dev, ferruh.yigit, ajit.khaparde, johndale, konstantin.ananyev, jingjing.wu, beilei.xing, jing.d.chen, adrien.mazarguil, nelio.laranjeiro, alejandro.lucero, hemant.agrawal, rasesh.mody, harish.patil, skhare, yliu, maxime.coquelin, allain.legacy One more comment below 01/09/2017 00:04, Thomas Monjalon: > Hi, > > 25/08/2017 15:47, David Harton: > > Some devices may not support or fail setting VLAN offload > > configuration based on dynamic circurmstances so the > > vlan_offload_set_t vector is modified to return an int so > > the caller can determine success or not. > > I agree with allowing to return an error. > > Comments on details below. > > The title could be changed to better reflect the purpose: > ethdev: allow returning error on VLAN configuration > > > --- a/doc/guides/rel_notes/release_17_11.rst > > +++ b/doc/guides/rel_notes/release_17_11.rst > > @@ -124,7 +124,7 @@ ABI Changes > > Also, make sure to start the actual text at the margin. > > ========================================================= > > > > - > > +* Changed return type of ``vlan_offload_set_t`` from ``void`` to ``int``. > > It should be referenced as an API change (instead of ABI change). > (and line spacing must be kept) > > We also need to bump the library version but it can be done with > bigger changes in ethdev. > > > --- a/lib/librte_ether/rte_ethdev.c > > +++ b/lib/librte_ether/rte_ethdev.c > > @@ -2049,10 +2049,16 @@ struct rte_eth_dev * > > int ret = 0; > > int mask = 0; > > int cur, org = 0; > > + uint8_t org_strip, org_filter, org_extend; > > > > RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV); > > dev = &rte_eth_devices[port_id]; > > > > + /* save original values in case of failure */ > > + org_strip = dev->data->dev_conf.rxmode.hw_vlan_strip; > > + org_filter = dev->data->dev_conf.rxmode.hw_vlan_filter; > > + org_extend = dev->data->dev_conf.rxmode.hw_vlan_extend; > > Please waste one more char to write "orig" instead of "org". > > > - (*dev->dev_ops->vlan_offload_set)(dev, mask); > > + ret = (*dev->dev_ops->vlan_offload_set)(dev, mask); Which error codes can be returned by this op? It is adding new error codes to rte_eth_dev_set_vlan_offload(), and they must be documented in the doxygen. > > + if (ret) { > > + /* hit an error restore original values */ > > + dev->data->dev_conf.rxmode.hw_vlan_strip = org_strip; > > + dev->data->dev_conf.rxmode.hw_vlan_filter = org_filter; > > + dev->data->dev_conf.rxmode.hw_vlan_extend = org_extend; > > + } > > Isn't it the responsibility of the PMD to restore values in case of error? > I understand it is there to factorize error handling code, right? > Do we want to document this behaviour with the ops prototype? Thanks ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [dpdk-dev] [PATCH v3] ethdev: modifiy vlan_offload_set_t to return int 2017-08-31 22:04 ` Thomas Monjalon 2017-08-31 22:08 ` Thomas Monjalon @ 2017-09-01 0:40 ` David Harton (dharton) 2017-09-01 8:01 ` Thomas Monjalon 1 sibling, 1 reply; 19+ messages in thread From: David Harton (dharton) @ 2017-09-01 0:40 UTC (permalink / raw) To: Thomas Monjalon Cc: dev, ferruh.yigit, ajit.khaparde, John Daley (johndale), konstantin.ananyev, jingjing.wu, beilei.xing, jing.d.chen, adrien.mazarguil, nelio.laranjeiro, alejandro.lucero, hemant.agrawal, rasesh.mody, harish.patil, skhare, yliu, maxime.coquelin, allain.legacy Hi Thomas, > -----Original Message----- > From: Thomas Monjalon [mailto:thomas@monjalon.net] > Sent: Thursday, August 31, 2017 6:04 PM > To: David Harton (dharton) <dharton@cisco.com> > Cc: dev@dpdk.org; ferruh.yigit@intel.com; ajit.khaparde@broadcom.com; John > Daley (johndale) <johndale@cisco.com>; konstantin.ananyev@intel.com; > jingjing.wu@intel.com; beilei.xing@intel.com; jing.d.chen@intel.com; > adrien.mazarguil@6wind.com; nelio.laranjeiro@6wind.com; > alejandro.lucero@netronome.com; hemant.agrawal@nxp.com; > rasesh.mody@cavium.com; harish.patil@cavium.com; skhare@vmware.com; > yliu@fridaylinux.org; maxime.coquelin@redhat.com; > allain.legacy@windriver.com > Subject: Re: [dpdk-dev] [PATCH v3] ethdev: modifiy vlan_offload_set_t to > return int > > Hi, > > 25/08/2017 15:47, David Harton: > > Some devices may not support or fail setting VLAN offload > > configuration based on dynamic circurmstances so the > > vlan_offload_set_t vector is modified to return an int so > > the caller can determine success or not. > > I agree with allowing to return an error. > > Comments on details below. > > The title could be changed to better reflect the purpose: > ethdev: allow returning error on VLAN configuration Sure. > > > --- a/doc/guides/rel_notes/release_17_11.rst > > +++ b/doc/guides/rel_notes/release_17_11.rst > > @@ -124,7 +124,7 @@ ABI Changes > > Also, make sure to start the actual text at the margin. > > ========================================================= > > > > - > > +* Changed return type of ``vlan_offload_set_t`` from ``void`` to ``int``. > > It should be referenced as an API change (instead of ABI change). > (and line spacing must be kept) Sure I'll move it to API. Line spacing? You mean 80 char width? I followed an example from a previous release so I'm not sure what you mean. > > We also need to bump the library version but it can be done with > bigger changes in ethdev. Ok. > > > --- a/lib/librte_ether/rte_ethdev.c > > +++ b/lib/librte_ether/rte_ethdev.c > > @@ -2049,10 +2049,16 @@ struct rte_eth_dev * > > int ret = 0; > > int mask = 0; > > int cur, org = 0; > > + uint8_t org_strip, org_filter, org_extend; > > > > RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV); > > dev = &rte_eth_devices[port_id]; > > > > + /* save original values in case of failure */ > > + org_strip = dev->data->dev_conf.rxmode.hw_vlan_strip; > > + org_filter = dev->data->dev_conf.rxmode.hw_vlan_filter; > > + org_extend = dev->data->dev_conf.rxmode.hw_vlan_extend; > > Please waste one more char to write "orig" instead of "org". Will do. > > > - (*dev->dev_ops->vlan_offload_set)(dev, mask); > > + ret = (*dev->dev_ops->vlan_offload_set)(dev, mask); > > + if (ret) { > > + /* hit an error restore original values */ > > + dev->data->dev_conf.rxmode.hw_vlan_strip = org_strip; > > + dev->data->dev_conf.rxmode.hw_vlan_filter = org_filter; > > + dev->data->dev_conf.rxmode.hw_vlan_extend = org_extend; > > + } > > Isn't it the responsibility of the PMD to restore values in case of error? > I understand it is there to factorize error handling code, right? By setting the dev_conf.rxmode.hw_vlan_* fields this function is claiming ownership of that data and therefore it should be the one to reset it. > Do we want to document this behaviour with the ops prototype? Why? Shouldn't any function that doesn't do what it was asked to do leave the system in its original state and in fact if it doesn't that is when it should be documented what the behavior is? Otherwise every caller has to implement some cleanup code and would have to be given information to know how to perform that cleanup as well which seems more complicated. Thanks, Dave ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [dpdk-dev] [PATCH v3] ethdev: modifiy vlan_offload_set_t to return int 2017-09-01 0:40 ` David Harton (dharton) @ 2017-09-01 8:01 ` Thomas Monjalon 0 siblings, 0 replies; 19+ messages in thread From: Thomas Monjalon @ 2017-09-01 8:01 UTC (permalink / raw) To: David Harton (dharton) Cc: dev, ferruh.yigit, ajit.khaparde, johndale, konstantin.ananyev, jingjing.wu, beilei.xing, jing.d.chen, adrien.mazarguil, nelio.laranjeiro, alejandro.lucero, hemant.agrawal, rasesh.mody, harish.patil, skhare, yliu, maxime.coquelin, allain.legacy 01/09/2017 02:40, David Harton (dharton): > From: Thomas Monjalon [mailto:thomas@monjalon.net] > > > --- a/doc/guides/rel_notes/release_17_11.rst > > > +++ b/doc/guides/rel_notes/release_17_11.rst > > > @@ -124,7 +124,7 @@ ABI Changes > > > Also, make sure to start the actual text at the margin. > > > ========================================================= > > > > > > - > > > +* Changed return type of ``vlan_offload_set_t`` from ``void`` to ``int``. > > > > It should be referenced as an API change (instead of ABI change). > > (and line spacing must be kept) > > Sure I'll move it to API. > > Line spacing? You mean 80 char width? I followed an example from a previous > release so I'm not sure what you mean. I mean you must take care of the number of blank lines to have before and after a title (2 blank lines before a title). > > > - (*dev->dev_ops->vlan_offload_set)(dev, mask); > > > + ret = (*dev->dev_ops->vlan_offload_set)(dev, mask); > > > + if (ret) { > > > + /* hit an error restore original values */ > > > + dev->data->dev_conf.rxmode.hw_vlan_strip = org_strip; > > > + dev->data->dev_conf.rxmode.hw_vlan_filter = org_filter; > > > + dev->data->dev_conf.rxmode.hw_vlan_extend = org_extend; > > > + } > > > > Isn't it the responsibility of the PMD to restore values in case of error? > > I understand it is there to factorize error handling code, right? > > By setting the dev_conf.rxmode.hw_vlan_* fields this function is claiming > ownership of that data and therefore it should be the one to reset it. > > > Do we want to document this behaviour with the ops prototype? > > Why? Shouldn't any function that doesn't do what it was asked to do > leave the system in its original state and in fact if it doesn't that > is when it should be documented what the behavior is? Otherwise every > caller has to implement some cleanup code and would have to be given > information to know how to perform that cleanup as well which seems > more complicated. Sorry I overlooked the function. I thought these data were set by the PMD, that's why I was telling the cleanup should be done in the PMD. You can forget this comment :) ^ permalink raw reply [flat|nested] 19+ messages in thread
* [dpdk-dev] [PATCH v4] ethdev: allow returning error on VLAN offload configuration 2017-08-25 13:47 ` [dpdk-dev] [PATCH v3] " David Harton 2017-08-31 22:04 ` Thomas Monjalon @ 2017-09-01 2:36 ` David Harton 2017-09-01 7:41 ` Hemant Agrawal ` (2 more replies) 1 sibling, 3 replies; 19+ messages in thread From: David Harton @ 2017-09-01 2:36 UTC (permalink / raw) To: thomas, ferruh.yigit, ajit.khaparde, johndale, konstantin.ananyev, jingjing.wu, beilei.xing, jing.d.chen, adrien.mazarguil, nelio.laranjeiro, alejandro.lucero, hemant.agrawal, rasesh.mody, harish.patil, skhare, yliu, maxime.coquelin, allain.legacy Cc: dev, David Harton Some devices may not support or fail setting VLAN offload configuration based on dynamic circurmstances so the vlan_offload_set_t vector is modified to return an int so the caller can determine success or not. rte_eth_dev_set_vlan_offload is updated to return the value provided by the vector when called along with restoring the original offload configs on failure. Existing vlan_offload_set_t vectors are modified to return an int. Majority of cases return 0 but a few that actually can fail now return their failure codes. Finally, a vlan_offload_set_t vector is added to virtio to facilitate dynamically turning VLAN strip on or off. Signed-off-by: David Harton <dharton@cisco.com> --- v4 * Modified commit message heading * Moved rel_note comments from ABI to API section * Renamed locals of rte_eth_dev_set_vlan_offload from 'org*' to 'orig*' v3 * Fixed a format error. * Apologies...need to figure out why checkpatches.pl keeps saying valid patch when I've got soft tabs. v2 * Fixed a missed format error. * Removed vlan offload vector call casts and replaced with checks for return values. v1 * This is an ABI breakage that has been previously negotiated with Thomas and the proposed rel note change is included as well. doc/guides/rel_notes/release_17_11.rst | 8 +++++++- drivers/net/avp/avp_ethdev.c | 12 +++++++++--- drivers/net/bnxt/bnxt_ethdev.c | 9 ++++++--- drivers/net/dpaa2/dpaa2_ethdev.c | 13 ++++++++++--- drivers/net/e1000/em_ethdev.c | 12 +++++++++--- drivers/net/e1000/igb_ethdev.c | 12 +++++++++--- drivers/net/enic/enic_ethdev.c | 8 +++++--- drivers/net/fm10k/fm10k_ethdev.c | 3 ++- drivers/net/i40e/i40e_ethdev.c | 11 ++++++++--- drivers/net/i40e/i40e_ethdev_vf.c | 9 ++++++--- drivers/net/ixgbe/ixgbe_ethdev.c | 25 ++++++++++++++++++------- drivers/net/mlx5/mlx5.h | 2 +- drivers/net/mlx5/mlx5_vlan.c | 3 ++- drivers/net/nfp/nfp_net.c | 13 +++++++------ drivers/net/qede/qede_ethdev.c | 9 +++++++-- drivers/net/virtio/virtio_ethdev.c | 21 +++++++++++++++++++++ drivers/net/vmxnet3/vmxnet3_ethdev.c | 10 +++++++--- lib/librte_ether/rte_ethdev.c | 28 ++++++++++++++++++++-------- lib/librte_ether/rte_ethdev.h | 2 +- 19 files changed, 155 insertions(+), 55 deletions(-) diff --git a/doc/guides/rel_notes/release_17_11.rst b/doc/guides/rel_notes/release_17_11.rst index 170f4f9..22df4fd 100644 --- a/doc/guides/rel_notes/release_17_11.rst +++ b/doc/guides/rel_notes/release_17_11.rst @@ -110,6 +110,13 @@ API Changes Also, make sure to start the actual text at the margin. ========================================================= +* **Modified the vlan_offload_set_t function prototype in the ethdev library.** + + Changed the function prototype of ``vlan_offload_set_t``. The return value + has been changed from ``void`` to ``int`` so the caller to knows whether + the backing device supports the operation or if the operation was + successfully performed. + ABI Changes ----------- @@ -125,7 +132,6 @@ ABI Changes ========================================================= - Shared Library Versions ----------------------- diff --git a/drivers/net/avp/avp_ethdev.c b/drivers/net/avp/avp_ethdev.c index c746a0e..4011dfa 100644 --- a/drivers/net/avp/avp_ethdev.c +++ b/drivers/net/avp/avp_ethdev.c @@ -70,7 +70,7 @@ static int avp_dev_create(struct rte_pci_device *pci_dev, static void avp_dev_close(struct rte_eth_dev *dev); static void avp_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info); -static void avp_vlan_offload_set(struct rte_eth_dev *dev, int mask); +static int avp_vlan_offload_set(struct rte_eth_dev *dev, int mask); static int avp_dev_link_update(struct rte_eth_dev *dev, int wait_to_complete); static void avp_dev_promiscuous_enable(struct rte_eth_dev *dev); static void avp_dev_promiscuous_disable(struct rte_eth_dev *dev); @@ -2031,7 +2031,12 @@ struct avp_queue { mask = (ETH_VLAN_STRIP_MASK | ETH_VLAN_FILTER_MASK | ETH_VLAN_EXTEND_MASK); - avp_vlan_offload_set(eth_dev, mask); + ret = avp_vlan_offload_set(eth_dev, mask); + if (ret < 0) { + PMD_DRV_LOG(ERR, "VLAN offload set failed by host, ret=%d\n", + ret); + goto unlock; + } /* update device config */ memset(&config, 0, sizeof(config)); @@ -2214,7 +2219,7 @@ struct avp_queue { } } -static void +static int avp_vlan_offload_set(struct rte_eth_dev *eth_dev, int mask) { struct avp_dev *avp = AVP_DEV_PRIVATE_TO_HW(eth_dev->data->dev_private); @@ -2239,6 +2244,7 @@ struct avp_queue { if (eth_dev->data->dev_conf.rxmode.hw_vlan_extend) PMD_DRV_LOG(ERR, "VLAN extend offload not supported\n"); } + return 0; } static void diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c index c9d1122..547bd55 100644 --- a/drivers/net/bnxt/bnxt_ethdev.c +++ b/drivers/net/bnxt/bnxt_ethdev.c @@ -144,7 +144,7 @@ ETH_RSS_NONFRAG_IPV6_TCP | \ ETH_RSS_NONFRAG_IPV6_UDP) -static void bnxt_vlan_offload_set_op(struct rte_eth_dev *dev, int mask); +static int bnxt_vlan_offload_set_op(struct rte_eth_dev *dev, int mask); /***********************/ @@ -522,7 +522,9 @@ static int bnxt_dev_start_op(struct rte_eth_dev *eth_dev) vlan_mask |= ETH_VLAN_FILTER_MASK; if (eth_dev->data->dev_conf.rxmode.hw_vlan_strip) vlan_mask |= ETH_VLAN_STRIP_MASK; - bnxt_vlan_offload_set_op(eth_dev, vlan_mask); + rc = bnxt_vlan_offload_set_op(eth_dev, vlan_mask); + if (rc) + goto error; return 0; @@ -1275,7 +1277,7 @@ static int bnxt_vlan_filter_set_op(struct rte_eth_dev *eth_dev, return bnxt_del_vlan_filter(bp, vlan_id); } -static void +static int bnxt_vlan_offload_set_op(struct rte_eth_dev *dev, int mask) { struct bnxt *bp = (struct bnxt *)dev->data->dev_private; @@ -1307,6 +1309,7 @@ static int bnxt_vlan_filter_set_op(struct rte_eth_dev *eth_dev, if (mask & ETH_VLAN_EXTEND_MASK) RTE_LOG(ERR, PMD, "Extend VLAN Not supported\n"); + return 0; } static void diff --git a/drivers/net/dpaa2/dpaa2_ethdev.c b/drivers/net/dpaa2/dpaa2_ethdev.c index 429b3a0..3390cb3 100644 --- a/drivers/net/dpaa2/dpaa2_ethdev.c +++ b/drivers/net/dpaa2/dpaa2_ethdev.c @@ -138,7 +138,7 @@ return ret; } -static void +static int dpaa2_vlan_offload_set(struct rte_eth_dev *dev, int mask) { struct dpaa2_dev_priv *priv = dev->data->dev_private; @@ -158,6 +158,7 @@ RTE_LOG(ERR, PMD, "Unable to set vlan filter = %d\n", ret); } + return 0; } static int @@ -643,8 +644,14 @@ return ret; } /* VLAN Offload Settings */ - if (priv->max_vlan_filters) - dpaa2_vlan_offload_set(dev, ETH_VLAN_FILTER_MASK); + if (priv->max_vlan_filters) { + ret = dpaa2_vlan_offload_set(dev, ETH_VLAN_FILTER_MASK); + if (ret) { + PMD_INIT_LOG(ERR, "Error to dpaa2_vlan_offload_set:" + "code = %d\n", ret); + return ret; + } + } return 0; } diff --git a/drivers/net/e1000/em_ethdev.c b/drivers/net/e1000/em_ethdev.c index 3d4ab93..51f49d8 100644 --- a/drivers/net/e1000/em_ethdev.c +++ b/drivers/net/e1000/em_ethdev.c @@ -99,7 +99,7 @@ static int eth_em_interrupt_action(struct rte_eth_dev *dev, static int eth_em_vlan_filter_set(struct rte_eth_dev *dev, uint16_t vlan_id, int on); -static void eth_em_vlan_offload_set(struct rte_eth_dev *dev, int mask); +static int eth_em_vlan_offload_set(struct rte_eth_dev *dev, int mask); static void em_vlan_hw_filter_enable(struct rte_eth_dev *dev); static void em_vlan_hw_filter_disable(struct rte_eth_dev *dev); static void em_vlan_hw_strip_enable(struct rte_eth_dev *dev); @@ -668,7 +668,12 @@ static int eth_em_pci_remove(struct rte_pci_device *pci_dev) mask = ETH_VLAN_STRIP_MASK | ETH_VLAN_FILTER_MASK | \ ETH_VLAN_EXTEND_MASK; - eth_em_vlan_offload_set(dev, mask); + ret = eth_em_vlan_offload_set(dev, mask); + if (ret) { + PMD_INIT_LOG(ERR, "Unable to update vlan offload"); + em_dev_clear_queues(dev); + return ret; + } /* Set Interrupt Throttling Rate to maximum allowed value. */ E1000_WRITE_REG(hw, E1000_ITR, UINT16_MAX); @@ -1447,7 +1452,7 @@ static int eth_em_pci_remove(struct rte_pci_device *pci_dev) E1000_WRITE_REG(hw, E1000_CTRL, reg); } -static void +static int eth_em_vlan_offload_set(struct rte_eth_dev *dev, int mask) { if(mask & ETH_VLAN_STRIP_MASK){ @@ -1463,6 +1468,7 @@ static int eth_em_pci_remove(struct rte_pci_device *pci_dev) else em_vlan_hw_filter_disable(dev); } + return 0; } /* diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c index e4f7a9f..fa15ee9 100644 --- a/drivers/net/e1000/igb_ethdev.c +++ b/drivers/net/e1000/igb_ethdev.c @@ -157,7 +157,7 @@ static int eth_igb_vlan_filter_set(struct rte_eth_dev *dev, static int eth_igb_vlan_tpid_set(struct rte_eth_dev *dev, enum rte_vlan_type vlan_type, uint16_t tpid_id); -static void eth_igb_vlan_offload_set(struct rte_eth_dev *dev, int mask); +static int eth_igb_vlan_offload_set(struct rte_eth_dev *dev, int mask); static void igb_vlan_hw_filter_enable(struct rte_eth_dev *dev); static void igb_vlan_hw_filter_disable(struct rte_eth_dev *dev); @@ -1400,7 +1400,12 @@ static int eth_igbvf_pci_remove(struct rte_pci_device *pci_dev) */ mask = ETH_VLAN_STRIP_MASK | ETH_VLAN_FILTER_MASK | \ ETH_VLAN_EXTEND_MASK; - eth_igb_vlan_offload_set(dev, mask); + ret = eth_igb_vlan_offload_set(dev, mask); + if (ret) { + PMD_INIT_LOG(ERR, "Unable to set vlan offload"); + igb_dev_clear_queues(dev); + return ret; + } if (dev->data->dev_conf.rxmode.mq_mode == ETH_MQ_RX_VMDQ_ONLY) { /* Enable VLAN filter since VMDq always use VLAN filter */ @@ -2715,7 +2720,7 @@ static int eth_igbvf_xstats_get_names(__rte_unused struct rte_eth_dev *dev, 2 * VLAN_TAG_SIZE); } -static void +static int eth_igb_vlan_offload_set(struct rte_eth_dev *dev, int mask) { if(mask & ETH_VLAN_STRIP_MASK){ @@ -2738,6 +2743,7 @@ static int eth_igbvf_xstats_get_names(__rte_unused struct rte_eth_dev *dev, else igb_vlan_hw_extend_disable(dev); } + return 0; } diff --git a/drivers/net/enic/enic_ethdev.c b/drivers/net/enic/enic_ethdev.c index da8fec2..fc1eac2 100644 --- a/drivers/net/enic/enic_ethdev.c +++ b/drivers/net/enic/enic_ethdev.c @@ -347,7 +347,7 @@ static int enicpmd_vlan_filter_set(struct rte_eth_dev *eth_dev, return err; } -static void enicpmd_vlan_offload_set(struct rte_eth_dev *eth_dev, int mask) +static int enicpmd_vlan_offload_set(struct rte_eth_dev *eth_dev, int mask) { struct enic *enic = pmd_priv(eth_dev); @@ -371,6 +371,8 @@ static void enicpmd_vlan_offload_set(struct rte_eth_dev *eth_dev, int mask) dev_warning(enic, "Configuration of extended VLAN is not supported\n"); } + + return 0; } static int enicpmd_dev_configure(struct rte_eth_dev *eth_dev) @@ -392,9 +394,9 @@ static int enicpmd_dev_configure(struct rte_eth_dev *eth_dev) eth_dev->data->dev_conf.rxmode.split_hdr_size); } - enicpmd_vlan_offload_set(eth_dev, ETH_VLAN_STRIP_MASK); + ret = enicpmd_vlan_offload_set(eth_dev, ETH_VLAN_STRIP_MASK); enic->hw_ip_checksum = eth_dev->data->dev_conf.rxmode.hw_ip_checksum; - return 0; + return ret; } /* Start the device. diff --git a/drivers/net/fm10k/fm10k_ethdev.c b/drivers/net/fm10k/fm10k_ethdev.c index e60d3a3..f4626f7 100644 --- a/drivers/net/fm10k/fm10k_ethdev.c +++ b/drivers/net/fm10k/fm10k_ethdev.c @@ -1590,7 +1590,7 @@ static int fm10k_xstats_get_names(__rte_unused struct rte_eth_dev *dev, return 0; } -static void +static int fm10k_vlan_offload_set(struct rte_eth_dev *dev, int mask) { if (mask & ETH_VLAN_STRIP_MASK) { @@ -1609,6 +1609,7 @@ static int fm10k_xstats_get_names(__rte_unused struct rte_eth_dev *dev, if (!dev->data->dev_conf.rxmode.hw_vlan_filter) PMD_INIT_LOG(ERR, "VLAN filter is always on in fm10k"); } + return 0; } /* Add/Remove a MAC address, and update filters to main VSI */ diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c index 00b6082..d03a44b 100644 --- a/drivers/net/i40e/i40e_ethdev.c +++ b/drivers/net/i40e/i40e_ethdev.c @@ -278,7 +278,7 @@ static int i40e_vlan_filter_set(struct rte_eth_dev *dev, static int i40e_vlan_tpid_set(struct rte_eth_dev *dev, enum rte_vlan_type vlan_type, uint16_t tpid); -static void i40e_vlan_offload_set(struct rte_eth_dev *dev, int mask); +static int i40e_vlan_offload_set(struct rte_eth_dev *dev, int mask); static void i40e_vlan_strip_queue_set(struct rte_eth_dev *dev, uint16_t queue, int on); @@ -3130,7 +3130,7 @@ static int i40e_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev, return ret; } -static void +static int i40e_vlan_offload_set(struct rte_eth_dev *dev, int mask) { struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private); @@ -3163,6 +3163,7 @@ static int i40e_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev, else i40e_vsi_config_double_vlan(vsi, FALSE); } + return 0; } static void @@ -5216,7 +5217,11 @@ struct i40e_vsi * /* Apply vlan offload setting */ mask = ETH_VLAN_STRIP_MASK | ETH_VLAN_FILTER_MASK; - i40e_vlan_offload_set(dev, mask); + ret = i40e_vlan_offload_set(dev, mask); + if (ret) { + PMD_DRV_LOG(INFO, "Failed to update vlan offload"); + return ret; + } /* Apply double-vlan setting, not implemented yet */ diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c index f6d8293..f7fffc2 100644 --- a/drivers/net/i40e/i40e_ethdev_vf.c +++ b/drivers/net/i40e/i40e_ethdev_vf.c @@ -118,7 +118,7 @@ static int i40evf_dev_xstats_get_names(struct rte_eth_dev *dev, static void i40evf_dev_xstats_reset(struct rte_eth_dev *dev); static int i40evf_vlan_filter_set(struct rte_eth_dev *dev, uint16_t vlan_id, int on); -static void i40evf_vlan_offload_set(struct rte_eth_dev *dev, int mask); +static int i40evf_vlan_offload_set(struct rte_eth_dev *dev, int mask); static int i40evf_vlan_pvid_set(struct rte_eth_dev *dev, uint16_t pvid, int on); static void i40evf_dev_close(struct rte_eth_dev *dev); @@ -1634,7 +1634,9 @@ static int eth_i40evf_pci_remove(struct rte_pci_device *pci_dev) int ret; /* Apply vlan offload setting */ - i40evf_vlan_offload_set(dev, ETH_VLAN_STRIP_MASK); + ret = i40evf_vlan_offload_set(dev, ETH_VLAN_STRIP_MASK); + if (ret) + return ret; /* Apply pvid setting */ ret = i40evf_vlan_pvid_set(dev, data->dev_conf.txmode.pvid, @@ -1642,7 +1644,7 @@ static int eth_i40evf_pci_remove(struct rte_pci_device *pci_dev) return ret; } -static void +static int i40evf_vlan_offload_set(struct rte_eth_dev *dev, int mask) { struct rte_eth_conf *dev_conf = &dev->data->dev_conf; @@ -1655,6 +1657,7 @@ static int eth_i40evf_pci_remove(struct rte_pci_device *pci_dev) else i40evf_disable_vlan_strip(dev); } + return 0; } static int diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c index 22171d8..1ec5aaf 100644 --- a/drivers/net/ixgbe/ixgbe_ethdev.c +++ b/drivers/net/ixgbe/ixgbe_ethdev.c @@ -218,7 +218,7 @@ static void ixgbe_vlan_hw_strip_bitmap_set(struct rte_eth_dev *dev, uint16_t queue, bool on); static void ixgbe_vlan_strip_queue_set(struct rte_eth_dev *dev, uint16_t queue, int on); -static void ixgbe_vlan_offload_set(struct rte_eth_dev *dev, int mask); +static int ixgbe_vlan_offload_set(struct rte_eth_dev *dev, int mask); static void ixgbe_vlan_hw_strip_enable(struct rte_eth_dev *dev, uint16_t queue); static void ixgbe_vlan_hw_strip_disable(struct rte_eth_dev *dev, uint16_t queue); static void ixgbe_vlan_hw_extend_enable(struct rte_eth_dev *dev); @@ -274,7 +274,7 @@ static int ixgbevf_vlan_filter_set(struct rte_eth_dev *dev, uint16_t vlan_id, int on); static void ixgbevf_vlan_strip_queue_set(struct rte_eth_dev *dev, uint16_t queue, int on); -static void ixgbevf_vlan_offload_set(struct rte_eth_dev *dev, int mask); +static int ixgbevf_vlan_offload_set(struct rte_eth_dev *dev, int mask); static void ixgbevf_set_vfta_all(struct rte_eth_dev *dev, bool on); static int ixgbevf_dev_rx_queue_intr_enable(struct rte_eth_dev *dev, uint16_t queue_id); @@ -2125,7 +2125,7 @@ static int eth_ixgbevf_pci_remove(struct rte_pci_device *pci_dev) */ } -static void +static int ixgbe_vlan_offload_set(struct rte_eth_dev *dev, int mask) { if (mask & ETH_VLAN_STRIP_MASK) { @@ -2148,6 +2148,7 @@ static int eth_ixgbevf_pci_remove(struct rte_pci_device *pci_dev) else ixgbe_vlan_hw_extend_disable(dev); } + return 0; } static void @@ -2568,9 +2569,13 @@ static int eth_ixgbevf_pci_remove(struct rte_pci_device *pci_dev) goto error; } - mask = ETH_VLAN_STRIP_MASK | ETH_VLAN_FILTER_MASK | + mask = ETH_VLAN_STRIP_MASK | ETH_VLAN_FILTER_MASK | ETH_VLAN_EXTEND_MASK; - ixgbe_vlan_offload_set(dev, mask); + err = ixgbe_vlan_offload_set(dev, mask); + if (err) { + PMD_INIT_LOG(ERR, "Unable to set VLAN offload"); + goto error; + } if (dev->data->dev_conf.rxmode.mq_mode == ETH_MQ_RX_VMDQ_ONLY) { /* Enable vlan filtering for VMDq */ @@ -4993,7 +4998,12 @@ static int ixgbevf_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev, /* Set HW strip */ mask = ETH_VLAN_STRIP_MASK | ETH_VLAN_FILTER_MASK | ETH_VLAN_EXTEND_MASK; - ixgbevf_vlan_offload_set(dev, mask); + err = ixgbevf_vlan_offload_set(dev, mask); + if (err) { + PMD_INIT_LOG(ERR, "Unable to set VLAN offload (%d)", err); + ixgbe_dev_clear_queues(dev); + return err; + } ixgbevf_dev_rxtx_start(dev); @@ -5153,7 +5163,7 @@ static void ixgbevf_set_vfta_all(struct rte_eth_dev *dev, bool on) ixgbe_vlan_hw_strip_bitmap_set(dev, queue, on); } -static void +static int ixgbevf_vlan_offload_set(struct rte_eth_dev *dev, int mask) { struct ixgbe_hw *hw = @@ -5168,6 +5178,7 @@ static void ixgbevf_set_vfta_all(struct rte_eth_dev *dev, bool on) for (i = 0; i < hw->mac.max_rx_queues; i++) ixgbevf_vlan_strip_queue_set(dev, i, on); } + return 0; } int diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h index 43c5384..93e71be 100644 --- a/drivers/net/mlx5/mlx5.h +++ b/drivers/net/mlx5/mlx5.h @@ -283,7 +283,7 @@ int mlx5_xstats_get_names(struct rte_eth_dev *, /* mlx5_vlan.c */ int mlx5_vlan_filter_set(struct rte_eth_dev *, uint16_t, int); -void mlx5_vlan_offload_set(struct rte_eth_dev *, int); +int mlx5_vlan_offload_set(struct rte_eth_dev *, int); void mlx5_vlan_strip_queue_set(struct rte_eth_dev *, uint16_t, int); /* mlx5_trigger.c */ diff --git a/drivers/net/mlx5/mlx5_vlan.c b/drivers/net/mlx5/mlx5_vlan.c index 1b0fa40..7215909 100644 --- a/drivers/net/mlx5/mlx5_vlan.c +++ b/drivers/net/mlx5/mlx5_vlan.c @@ -210,7 +210,7 @@ * @param mask * VLAN offload bit mask. */ -void +int mlx5_vlan_offload_set(struct rte_eth_dev *dev, int mask) { struct priv *priv = dev->data->dev_private; @@ -230,4 +230,5 @@ priv_vlan_strip_queue_set(priv, i, hw_vlan_strip); priv_unlock(priv); } + return 0; } diff --git a/drivers/net/nfp/nfp_net.c b/drivers/net/nfp/nfp_net.c index 92b03c4..6473edc 100644 --- a/drivers/net/nfp/nfp_net.c +++ b/drivers/net/nfp/nfp_net.c @@ -2149,11 +2149,12 @@ uint32_t nfp_net_txq_full(struct nfp_net_txq *txq) return i; } -static void +static int nfp_net_vlan_offload_set(struct rte_eth_dev *dev, int mask) { uint32_t new_ctrl, update; struct nfp_net_hw *hw; + int ret; hw = NFP_NET_DEV_PRIVATE_TO_HW(dev->data->dev_private); new_ctrl = 0; @@ -2174,14 +2175,14 @@ uint32_t nfp_net_txq_full(struct nfp_net_txq *txq) new_ctrl = hw->ctrl & ~NFP_NET_CFG_CTRL_RXVLAN; if (new_ctrl == 0) - return; + return 0; update = NFP_NET_CFG_UPDATE_GEN; - if (nfp_net_reconfig(hw, new_ctrl, update) < 0) - return; - - hw->ctrl = new_ctrl; + ret = nfp_net_reconfig(hw, new_ctrl, update); + if (!ret) + hw->ctrl = new_ctrl; + return ret; } /* Update Redirection Table(RETA) of Receive Side Scaling of Ethernet device */ diff --git a/drivers/net/qede/qede_ethdev.c b/drivers/net/qede/qede_ethdev.c index 0e05989..644f69d 100644 --- a/drivers/net/qede/qede_ethdev.c +++ b/drivers/net/qede/qede_ethdev.c @@ -975,7 +975,7 @@ static int qede_vlan_filter_set(struct rte_eth_dev *eth_dev, return rc; } -static void qede_vlan_offload_set(struct rte_eth_dev *eth_dev, int mask) +static int qede_vlan_offload_set(struct rte_eth_dev *eth_dev, int mask) { struct qede_dev *qdev = QEDE_INIT_QDEV(eth_dev); struct ecore_dev *edev = QEDE_INIT_EDEV(qdev); @@ -1013,6 +1013,8 @@ static void qede_vlan_offload_set(struct rte_eth_dev *eth_dev, int mask) DP_INFO(edev, "vlan offload mask %d vlan-strip %d vlan-filter %d\n", mask, rxmode->hw_vlan_strip, rxmode->hw_vlan_filter); + + return 0; } static void qede_prandom_bytes(uint32_t *buff) @@ -1157,6 +1159,7 @@ static int qede_dev_configure(struct rte_eth_dev *eth_dev) struct qede_dev *qdev = QEDE_INIT_QDEV(eth_dev); struct ecore_dev *edev = QEDE_INIT_EDEV(qdev); struct rte_eth_rxmode *rxmode = ð_dev->data->dev_conf.rxmode; + int ret; PMD_INIT_FUNC_TRACE(edev); @@ -1237,9 +1240,11 @@ static int qede_dev_configure(struct rte_eth_dev *eth_dev) qdev->enable_lro = rxmode->enable_lro; /* Enable VLAN offloads by default */ - qede_vlan_offload_set(eth_dev, ETH_VLAN_STRIP_MASK | + ret = qede_vlan_offload_set(eth_dev, ETH_VLAN_STRIP_MASK | ETH_VLAN_FILTER_MASK | ETH_VLAN_EXTEND_MASK); + if (ret) + return ret; DP_INFO(edev, "Device configured with RSS=%d TSS=%d\n", QEDE_RSS_COUNT(qdev), QEDE_TSS_COUNT(qdev)); diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c index e320811..72b4248 100644 --- a/drivers/net/virtio/virtio_ethdev.c +++ b/drivers/net/virtio/virtio_ethdev.c @@ -72,6 +72,7 @@ static void virtio_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info); static int virtio_dev_link_update(struct rte_eth_dev *dev, int wait_to_complete); +static int virtio_dev_vlan_offload_set(struct rte_eth_dev *dev, int mask); static void virtio_set_hwaddr(struct virtio_hw *hw); static void virtio_get_hwaddr(struct virtio_hw *hw); @@ -779,6 +780,7 @@ struct rte_virtio_xstats_name_off { .stats_reset = virtio_dev_stats_reset, .xstats_reset = virtio_dev_stats_reset, .link_update = virtio_dev_link_update, + .vlan_offload_set = virtio_dev_vlan_offload_set, .rx_queue_setup = virtio_dev_rx_queue_setup, .rx_queue_intr_enable = virtio_dev_rx_queue_intr_enable, .rx_queue_intr_disable = virtio_dev_rx_queue_intr_disable, @@ -1875,6 +1877,25 @@ static void virtio_dev_free_mbufs(struct rte_eth_dev *dev) return (old.link_status == link.link_status) ? -1 : 0; } +static int +virtio_dev_vlan_offload_set(struct rte_eth_dev *dev, int mask) +{ + const struct rte_eth_rxmode *rxmode = &dev->data->dev_conf.rxmode; + struct virtio_hw *hw = dev->data->dev_private; + + if (rxmode->hw_vlan_filter && + !vtpci_with_feature(hw, VIRTIO_NET_F_CTRL_VLAN)) { + PMD_DRV_LOG(NOTICE, + "vlan filtering not available on this host"); + return -ENOTSUP; + } + + if (mask & ETH_VLAN_STRIP_MASK) + hw->vlan_strip = rxmode->hw_vlan_strip; + + return 0; +} + static void virtio_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info) { diff --git a/drivers/net/vmxnet3/vmxnet3_ethdev.c b/drivers/net/vmxnet3/vmxnet3_ethdev.c index 3910991..06735dd 100644 --- a/drivers/net/vmxnet3/vmxnet3_ethdev.c +++ b/drivers/net/vmxnet3/vmxnet3_ethdev.c @@ -100,7 +100,7 @@ static void vmxnet3_dev_info_get(struct rte_eth_dev *dev, 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 void vmxnet3_dev_vlan_offload_set(struct rte_eth_dev *dev, int mask); +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, struct ether_addr *mac_addr); static void vmxnet3_interrupt_handler(void *param); @@ -730,8 +730,10 @@ static int eth_vmxnet3_pci_remove(struct rte_pci_device *pci_dev) devRead->rssConfDesc.confPA = hw->rss_confPA; } - vmxnet3_dev_vlan_offload_set(dev, + ret = vmxnet3_dev_vlan_offload_set(dev, ETH_VLAN_STRIP_MASK | ETH_VLAN_FILTER_MASK); + if (ret) + return ret; vmxnet3_write_mac(hw, dev->data->mac_addrs->addr_bytes); @@ -1275,7 +1277,7 @@ static int eth_vmxnet3_pci_remove(struct rte_pci_device *pci_dev) return 0; } -static void +static int vmxnet3_dev_vlan_offload_set(struct rte_eth_dev *dev, int mask) { struct vmxnet3_hw *hw = dev->data->dev_private; @@ -1301,6 +1303,8 @@ static int eth_vmxnet3_pci_remove(struct rte_pci_device *pci_dev) VMXNET3_WRITE_BAR1_REG(hw, VMXNET3_REG_CMD, VMXNET3_CMD_UPDATE_VLAN_FILTERS); } + + return 0; } static void diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c index 0597641..05e52b8 100644 --- a/lib/librte_ether/rte_ethdev.c +++ b/lib/librte_ether/rte_ethdev.c @@ -2048,29 +2048,35 @@ struct rte_eth_dev * struct rte_eth_dev *dev; int ret = 0; int mask = 0; - int cur, org = 0; + int cur, orig = 0; + uint8_t orig_strip, orig_filter, orig_extend; RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV); dev = &rte_eth_devices[port_id]; + /* save original values in case of failure */ + orig_strip = dev->data->dev_conf.rxmode.hw_vlan_strip; + orig_filter = dev->data->dev_conf.rxmode.hw_vlan_filter; + orig_extend = dev->data->dev_conf.rxmode.hw_vlan_extend; + /*check which option changed by application*/ cur = !!(offload_mask & ETH_VLAN_STRIP_OFFLOAD); - org = !!(dev->data->dev_conf.rxmode.hw_vlan_strip); - if (cur != org) { + orig = !!(dev->data->dev_conf.rxmode.hw_vlan_strip); + if (cur != orig) { dev->data->dev_conf.rxmode.hw_vlan_strip = (uint8_t)cur; mask |= ETH_VLAN_STRIP_MASK; } cur = !!(offload_mask & ETH_VLAN_FILTER_OFFLOAD); - org = !!(dev->data->dev_conf.rxmode.hw_vlan_filter); - if (cur != org) { + orig = !!(dev->data->dev_conf.rxmode.hw_vlan_filter); + if (cur != orig) { dev->data->dev_conf.rxmode.hw_vlan_filter = (uint8_t)cur; mask |= ETH_VLAN_FILTER_MASK; } cur = !!(offload_mask & ETH_VLAN_EXTEND_OFFLOAD); - org = !!(dev->data->dev_conf.rxmode.hw_vlan_extend); - if (cur != org) { + orig = !!(dev->data->dev_conf.rxmode.hw_vlan_extend); + if (cur != orig) { dev->data->dev_conf.rxmode.hw_vlan_extend = (uint8_t)cur; mask |= ETH_VLAN_EXTEND_MASK; } @@ -2080,7 +2086,13 @@ struct rte_eth_dev * return ret; RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->vlan_offload_set, -ENOTSUP); - (*dev->dev_ops->vlan_offload_set)(dev, mask); + ret = (*dev->dev_ops->vlan_offload_set)(dev, mask); + if (ret) { + /* hit an error restore original values */ + dev->data->dev_conf.rxmode.hw_vlan_strip = orig_strip; + dev->data->dev_conf.rxmode.hw_vlan_filter = orig_filter; + dev->data->dev_conf.rxmode.hw_vlan_extend = orig_extend; + } return ret; } diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h index 0adf327..7254fd0 100644 --- a/lib/librte_ether/rte_ethdev.h +++ b/lib/librte_ether/rte_ethdev.h @@ -1245,7 +1245,7 @@ typedef int (*vlan_tpid_set_t)(struct rte_eth_dev *dev, enum rte_vlan_type type, uint16_t tpid); /**< @internal set the outer/inner VLAN-TPID by an Ethernet device. */ -typedef void (*vlan_offload_set_t)(struct rte_eth_dev *dev, int mask); +typedef int (*vlan_offload_set_t)(struct rte_eth_dev *dev, int mask); /**< @internal set VLAN offload function by an Ethernet device. */ typedef int (*vlan_pvid_set_t)(struct rte_eth_dev *dev, -- 1.8.3.1 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [dpdk-dev] [PATCH v4] ethdev: allow returning error on VLAN offload configuration 2017-09-01 2:36 ` [dpdk-dev] [PATCH v4] ethdev: allow returning error on VLAN offload configuration David Harton @ 2017-09-01 7:41 ` Hemant Agrawal 2017-09-01 12:54 ` David Harton (dharton) 2017-10-10 5:34 ` Ferruh Yigit 2017-10-25 3:01 ` [dpdk-dev] [PATCH v5] ethdev: allow returning error on VLAN offload ops Ferruh Yigit 2 siblings, 1 reply; 19+ messages in thread From: Hemant Agrawal @ 2017-09-01 7:41 UTC (permalink / raw) To: David Harton, thomas, ferruh.yigit, ajit.khaparde, johndale, konstantin.ananyev, jingjing.wu, beilei.xing, jing.d.chen, adrien.mazarguil, nelio.laranjeiro, alejandro.lucero, rasesh.mody, harish.patil, skhare, yliu, maxime.coquelin, allain.legacy Cc: dev On 9/1/2017 8:06 AM, David Harton wrote: > Some devices may not support or fail setting VLAN offload > configuration based on dynamic circurmstances so the > vlan_offload_set_t vector is modified to return an int so > the caller can determine success or not. > > rte_eth_dev_set_vlan_offload is updated to return the > value provided by the vector when called along with restoring > the original offload configs on failure. > > Existing vlan_offload_set_t vectors are modified to return > an int. Majority of cases return 0 but a few that actually > can fail now return their failure codes. > > Finally, a vlan_offload_set_t vector is added to virtio > to facilitate dynamically turning VLAN strip on or off. > > Signed-off-by: David Harton <dharton@cisco.com> > --- > > v4 > * Modified commit message heading > * Moved rel_note comments from ABI to API section > * Renamed locals of rte_eth_dev_set_vlan_offload from 'org*' to 'orig*' > > v3 > * Fixed a format error. > * Apologies...need to figure out why checkpatches.pl keeps saying > valid patch when I've got soft tabs. > > v2 > * Fixed a missed format error. > * Removed vlan offload vector call casts and replaced with checks > for return values. > > v1 > * This is an ABI breakage that has been previously negotiated > with Thomas and the proposed rel note change is included as well. > > doc/guides/rel_notes/release_17_11.rst | 8 +++++++- > drivers/net/avp/avp_ethdev.c | 12 +++++++++--- > drivers/net/bnxt/bnxt_ethdev.c | 9 ++++++--- > drivers/net/dpaa2/dpaa2_ethdev.c | 13 ++++++++++--- > drivers/net/e1000/em_ethdev.c | 12 +++++++++--- > drivers/net/e1000/igb_ethdev.c | 12 +++++++++--- > drivers/net/enic/enic_ethdev.c | 8 +++++--- > drivers/net/fm10k/fm10k_ethdev.c | 3 ++- > drivers/net/i40e/i40e_ethdev.c | 11 ++++++++--- > drivers/net/i40e/i40e_ethdev_vf.c | 9 ++++++--- > drivers/net/ixgbe/ixgbe_ethdev.c | 25 ++++++++++++++++++------- > drivers/net/mlx5/mlx5.h | 2 +- > drivers/net/mlx5/mlx5_vlan.c | 3 ++- > drivers/net/nfp/nfp_net.c | 13 +++++++------ > drivers/net/qede/qede_ethdev.c | 9 +++++++-- > drivers/net/virtio/virtio_ethdev.c | 21 +++++++++++++++++++++ > drivers/net/vmxnet3/vmxnet3_ethdev.c | 10 +++++++--- > lib/librte_ether/rte_ethdev.c | 28 ++++++++++++++++++++-------- > lib/librte_ether/rte_ethdev.h | 2 +- > 19 files changed, 155 insertions(+), 55 deletions(-) > > diff --git a/doc/guides/rel_notes/release_17_11.rst b/doc/guides/rel_notes/release_17_11.rst > index 170f4f9..22df4fd 100644 > --- a/doc/guides/rel_notes/release_17_11.rst > +++ b/doc/guides/rel_notes/release_17_11.rst > @@ -110,6 +110,13 @@ API Changes > Also, make sure to start the actual text at the margin. > ========================================================= > > +* **Modified the vlan_offload_set_t function prototype in the ethdev library.** > + > + Changed the function prototype of ``vlan_offload_set_t``. The return value > + has been changed from ``void`` to ``int`` so the caller to knows whether > + the backing device supports the operation or if the operation was > + successfully performed. > + > > ABI Changes > ----------- > @@ -125,7 +132,6 @@ ABI Changes > ========================================================= > > > - > Shared Library Versions > ----------------------- > > diff --git a/drivers/net/avp/avp_ethdev.c b/drivers/net/avp/avp_ethdev.c > index c746a0e..4011dfa 100644 > --- a/drivers/net/avp/avp_ethdev.c > +++ b/drivers/net/avp/avp_ethdev.c > @@ -70,7 +70,7 @@ static int avp_dev_create(struct rte_pci_device *pci_dev, > static void avp_dev_close(struct rte_eth_dev *dev); > static void avp_dev_info_get(struct rte_eth_dev *dev, > struct rte_eth_dev_info *dev_info); > -static void avp_vlan_offload_set(struct rte_eth_dev *dev, int mask); > +static int avp_vlan_offload_set(struct rte_eth_dev *dev, int mask); > static int avp_dev_link_update(struct rte_eth_dev *dev, int wait_to_complete); > static void avp_dev_promiscuous_enable(struct rte_eth_dev *dev); > static void avp_dev_promiscuous_disable(struct rte_eth_dev *dev); > @@ -2031,7 +2031,12 @@ struct avp_queue { > mask = (ETH_VLAN_STRIP_MASK | > ETH_VLAN_FILTER_MASK | > ETH_VLAN_EXTEND_MASK); > - avp_vlan_offload_set(eth_dev, mask); > + ret = avp_vlan_offload_set(eth_dev, mask); > + if (ret < 0) { > + PMD_DRV_LOG(ERR, "VLAN offload set failed by host, ret=%d\n", > + ret); > + goto unlock; > + } > > /* update device config */ > memset(&config, 0, sizeof(config)); > @@ -2214,7 +2219,7 @@ struct avp_queue { > } > } > > -static void > +static int > avp_vlan_offload_set(struct rte_eth_dev *eth_dev, int mask) > { > struct avp_dev *avp = AVP_DEV_PRIVATE_TO_HW(eth_dev->data->dev_private); > @@ -2239,6 +2244,7 @@ struct avp_queue { > if (eth_dev->data->dev_conf.rxmode.hw_vlan_extend) > PMD_DRV_LOG(ERR, "VLAN extend offload not supported\n"); > } > + return 0; > } > > static void > diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c > index c9d1122..547bd55 100644 > --- a/drivers/net/bnxt/bnxt_ethdev.c > +++ b/drivers/net/bnxt/bnxt_ethdev.c > @@ -144,7 +144,7 @@ > ETH_RSS_NONFRAG_IPV6_TCP | \ > ETH_RSS_NONFRAG_IPV6_UDP) > > -static void bnxt_vlan_offload_set_op(struct rte_eth_dev *dev, int mask); > +static int bnxt_vlan_offload_set_op(struct rte_eth_dev *dev, int mask); > > /***********************/ > > @@ -522,7 +522,9 @@ static int bnxt_dev_start_op(struct rte_eth_dev *eth_dev) > vlan_mask |= ETH_VLAN_FILTER_MASK; > if (eth_dev->data->dev_conf.rxmode.hw_vlan_strip) > vlan_mask |= ETH_VLAN_STRIP_MASK; > - bnxt_vlan_offload_set_op(eth_dev, vlan_mask); > + rc = bnxt_vlan_offload_set_op(eth_dev, vlan_mask); > + if (rc) > + goto error; > > return 0; > > @@ -1275,7 +1277,7 @@ static int bnxt_vlan_filter_set_op(struct rte_eth_dev *eth_dev, > return bnxt_del_vlan_filter(bp, vlan_id); > } > > -static void > +static int > bnxt_vlan_offload_set_op(struct rte_eth_dev *dev, int mask) > { > struct bnxt *bp = (struct bnxt *)dev->data->dev_private; > @@ -1307,6 +1309,7 @@ static int bnxt_vlan_filter_set_op(struct rte_eth_dev *eth_dev, > > if (mask & ETH_VLAN_EXTEND_MASK) > RTE_LOG(ERR, PMD, "Extend VLAN Not supported\n"); > + return 0; > } > > static void > diff --git a/drivers/net/dpaa2/dpaa2_ethdev.c b/drivers/net/dpaa2/dpaa2_ethdev.c > index 429b3a0..3390cb3 100644 > --- a/drivers/net/dpaa2/dpaa2_ethdev.c > +++ b/drivers/net/dpaa2/dpaa2_ethdev.c > @@ -138,7 +138,7 @@ > return ret; > } > > -static void > +static int > dpaa2_vlan_offload_set(struct rte_eth_dev *dev, int mask) > { > struct dpaa2_dev_priv *priv = dev->data->dev_private; > @@ -158,6 +158,7 @@ > RTE_LOG(ERR, PMD, "Unable to set vlan filter = %d\n", > ret); > } > + return 0; > } > > static int > @@ -643,8 +644,14 @@ > return ret; > } > /* VLAN Offload Settings */ > - if (priv->max_vlan_filters) > - dpaa2_vlan_offload_set(dev, ETH_VLAN_FILTER_MASK); > + if (priv->max_vlan_filters) { > + ret = dpaa2_vlan_offload_set(dev, ETH_VLAN_FILTER_MASK); > + if (ret) { > + PMD_INIT_LOG(ERR, "Error to dpaa2_vlan_offload_set:" > + "code = %d\n", ret); > + return ret; > + } > + } > > return 0; > } > diff --git a/drivers/net/e1000/em_ethdev.c b/drivers/net/e1000/em_ethdev.c > index 3d4ab93..51f49d8 100644 > --- a/drivers/net/e1000/em_ethdev.c > +++ b/drivers/net/e1000/em_ethdev.c > @@ -99,7 +99,7 @@ static int eth_em_interrupt_action(struct rte_eth_dev *dev, > > static int eth_em_vlan_filter_set(struct rte_eth_dev *dev, > uint16_t vlan_id, int on); > -static void eth_em_vlan_offload_set(struct rte_eth_dev *dev, int mask); > +static int eth_em_vlan_offload_set(struct rte_eth_dev *dev, int mask); > static void em_vlan_hw_filter_enable(struct rte_eth_dev *dev); > static void em_vlan_hw_filter_disable(struct rte_eth_dev *dev); > static void em_vlan_hw_strip_enable(struct rte_eth_dev *dev); > @@ -668,7 +668,12 @@ static int eth_em_pci_remove(struct rte_pci_device *pci_dev) > > mask = ETH_VLAN_STRIP_MASK | ETH_VLAN_FILTER_MASK | \ > ETH_VLAN_EXTEND_MASK; > - eth_em_vlan_offload_set(dev, mask); > + ret = eth_em_vlan_offload_set(dev, mask); > + if (ret) { > + PMD_INIT_LOG(ERR, "Unable to update vlan offload"); > + em_dev_clear_queues(dev); > + return ret; > + } > > /* Set Interrupt Throttling Rate to maximum allowed value. */ > E1000_WRITE_REG(hw, E1000_ITR, UINT16_MAX); > @@ -1447,7 +1452,7 @@ static int eth_em_pci_remove(struct rte_pci_device *pci_dev) > E1000_WRITE_REG(hw, E1000_CTRL, reg); > } > > -static void > +static int > eth_em_vlan_offload_set(struct rte_eth_dev *dev, int mask) > { > if(mask & ETH_VLAN_STRIP_MASK){ > @@ -1463,6 +1468,7 @@ static int eth_em_pci_remove(struct rte_pci_device *pci_dev) > else > em_vlan_hw_filter_disable(dev); > } > + return 0; > } > > /* > diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c > index e4f7a9f..fa15ee9 100644 > --- a/drivers/net/e1000/igb_ethdev.c > +++ b/drivers/net/e1000/igb_ethdev.c > @@ -157,7 +157,7 @@ static int eth_igb_vlan_filter_set(struct rte_eth_dev *dev, > static int eth_igb_vlan_tpid_set(struct rte_eth_dev *dev, > enum rte_vlan_type vlan_type, > uint16_t tpid_id); > -static void eth_igb_vlan_offload_set(struct rte_eth_dev *dev, int mask); > +static int eth_igb_vlan_offload_set(struct rte_eth_dev *dev, int mask); > > static void igb_vlan_hw_filter_enable(struct rte_eth_dev *dev); > static void igb_vlan_hw_filter_disable(struct rte_eth_dev *dev); > @@ -1400,7 +1400,12 @@ static int eth_igbvf_pci_remove(struct rte_pci_device *pci_dev) > */ > mask = ETH_VLAN_STRIP_MASK | ETH_VLAN_FILTER_MASK | \ > ETH_VLAN_EXTEND_MASK; > - eth_igb_vlan_offload_set(dev, mask); > + ret = eth_igb_vlan_offload_set(dev, mask); > + if (ret) { > + PMD_INIT_LOG(ERR, "Unable to set vlan offload"); > + igb_dev_clear_queues(dev); > + return ret; > + } > > if (dev->data->dev_conf.rxmode.mq_mode == ETH_MQ_RX_VMDQ_ONLY) { > /* Enable VLAN filter since VMDq always use VLAN filter */ > @@ -2715,7 +2720,7 @@ static int eth_igbvf_xstats_get_names(__rte_unused struct rte_eth_dev *dev, > 2 * VLAN_TAG_SIZE); > } > > -static void > +static int > eth_igb_vlan_offload_set(struct rte_eth_dev *dev, int mask) > { > if(mask & ETH_VLAN_STRIP_MASK){ > @@ -2738,6 +2743,7 @@ static int eth_igbvf_xstats_get_names(__rte_unused struct rte_eth_dev *dev, > else > igb_vlan_hw_extend_disable(dev); > } > + return 0; > } > > > diff --git a/drivers/net/enic/enic_ethdev.c b/drivers/net/enic/enic_ethdev.c > index da8fec2..fc1eac2 100644 > --- a/drivers/net/enic/enic_ethdev.c > +++ b/drivers/net/enic/enic_ethdev.c > @@ -347,7 +347,7 @@ static int enicpmd_vlan_filter_set(struct rte_eth_dev *eth_dev, > return err; > } > > -static void enicpmd_vlan_offload_set(struct rte_eth_dev *eth_dev, int mask) > +static int enicpmd_vlan_offload_set(struct rte_eth_dev *eth_dev, int mask) > { > struct enic *enic = pmd_priv(eth_dev); > > @@ -371,6 +371,8 @@ static void enicpmd_vlan_offload_set(struct rte_eth_dev *eth_dev, int mask) > dev_warning(enic, > "Configuration of extended VLAN is not supported\n"); > } > + > + return 0; > } > > static int enicpmd_dev_configure(struct rte_eth_dev *eth_dev) > @@ -392,9 +394,9 @@ static int enicpmd_dev_configure(struct rte_eth_dev *eth_dev) > eth_dev->data->dev_conf.rxmode.split_hdr_size); > } > > - enicpmd_vlan_offload_set(eth_dev, ETH_VLAN_STRIP_MASK); > + ret = enicpmd_vlan_offload_set(eth_dev, ETH_VLAN_STRIP_MASK); > enic->hw_ip_checksum = eth_dev->data->dev_conf.rxmode.hw_ip_checksum; > - return 0; > + return ret; > } > > /* Start the device. > diff --git a/drivers/net/fm10k/fm10k_ethdev.c b/drivers/net/fm10k/fm10k_ethdev.c > index e60d3a3..f4626f7 100644 > --- a/drivers/net/fm10k/fm10k_ethdev.c > +++ b/drivers/net/fm10k/fm10k_ethdev.c > @@ -1590,7 +1590,7 @@ static int fm10k_xstats_get_names(__rte_unused struct rte_eth_dev *dev, > return 0; > } > > -static void > +static int > fm10k_vlan_offload_set(struct rte_eth_dev *dev, int mask) > { > if (mask & ETH_VLAN_STRIP_MASK) { > @@ -1609,6 +1609,7 @@ static int fm10k_xstats_get_names(__rte_unused struct rte_eth_dev *dev, > if (!dev->data->dev_conf.rxmode.hw_vlan_filter) > PMD_INIT_LOG(ERR, "VLAN filter is always on in fm10k"); > } > + return 0; > } > > /* Add/Remove a MAC address, and update filters to main VSI */ > diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c > index 00b6082..d03a44b 100644 > --- a/drivers/net/i40e/i40e_ethdev.c > +++ b/drivers/net/i40e/i40e_ethdev.c > @@ -278,7 +278,7 @@ static int i40e_vlan_filter_set(struct rte_eth_dev *dev, > static int i40e_vlan_tpid_set(struct rte_eth_dev *dev, > enum rte_vlan_type vlan_type, > uint16_t tpid); > -static void i40e_vlan_offload_set(struct rte_eth_dev *dev, int mask); > +static int i40e_vlan_offload_set(struct rte_eth_dev *dev, int mask); > static void i40e_vlan_strip_queue_set(struct rte_eth_dev *dev, > uint16_t queue, > int on); > @@ -3130,7 +3130,7 @@ static int i40e_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev, > return ret; > } > > -static void > +static int > i40e_vlan_offload_set(struct rte_eth_dev *dev, int mask) > { > struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private); > @@ -3163,6 +3163,7 @@ static int i40e_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev, > else > i40e_vsi_config_double_vlan(vsi, FALSE); > } > + return 0; > } > > static void > @@ -5216,7 +5217,11 @@ struct i40e_vsi * > > /* Apply vlan offload setting */ > mask = ETH_VLAN_STRIP_MASK | ETH_VLAN_FILTER_MASK; > - i40e_vlan_offload_set(dev, mask); > + ret = i40e_vlan_offload_set(dev, mask); > + if (ret) { > + PMD_DRV_LOG(INFO, "Failed to update vlan offload"); > + return ret; > + } > > /* Apply double-vlan setting, not implemented yet */ > > diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c > index f6d8293..f7fffc2 100644 > --- a/drivers/net/i40e/i40e_ethdev_vf.c > +++ b/drivers/net/i40e/i40e_ethdev_vf.c > @@ -118,7 +118,7 @@ static int i40evf_dev_xstats_get_names(struct rte_eth_dev *dev, > static void i40evf_dev_xstats_reset(struct rte_eth_dev *dev); > static int i40evf_vlan_filter_set(struct rte_eth_dev *dev, > uint16_t vlan_id, int on); > -static void i40evf_vlan_offload_set(struct rte_eth_dev *dev, int mask); > +static int i40evf_vlan_offload_set(struct rte_eth_dev *dev, int mask); > static int i40evf_vlan_pvid_set(struct rte_eth_dev *dev, uint16_t pvid, > int on); > static void i40evf_dev_close(struct rte_eth_dev *dev); > @@ -1634,7 +1634,9 @@ static int eth_i40evf_pci_remove(struct rte_pci_device *pci_dev) > int ret; > > /* Apply vlan offload setting */ > - i40evf_vlan_offload_set(dev, ETH_VLAN_STRIP_MASK); > + ret = i40evf_vlan_offload_set(dev, ETH_VLAN_STRIP_MASK); > + if (ret) > + return ret; > > /* Apply pvid setting */ > ret = i40evf_vlan_pvid_set(dev, data->dev_conf.txmode.pvid, > @@ -1642,7 +1644,7 @@ static int eth_i40evf_pci_remove(struct rte_pci_device *pci_dev) > return ret; > } > > -static void > +static int > i40evf_vlan_offload_set(struct rte_eth_dev *dev, int mask) > { > struct rte_eth_conf *dev_conf = &dev->data->dev_conf; > @@ -1655,6 +1657,7 @@ static int eth_i40evf_pci_remove(struct rte_pci_device *pci_dev) > else > i40evf_disable_vlan_strip(dev); > } > + return 0; > } > > static int > diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c > index 22171d8..1ec5aaf 100644 > --- a/drivers/net/ixgbe/ixgbe_ethdev.c > +++ b/drivers/net/ixgbe/ixgbe_ethdev.c > @@ -218,7 +218,7 @@ static void ixgbe_vlan_hw_strip_bitmap_set(struct rte_eth_dev *dev, > uint16_t queue, bool on); > static void ixgbe_vlan_strip_queue_set(struct rte_eth_dev *dev, uint16_t queue, > int on); > -static void ixgbe_vlan_offload_set(struct rte_eth_dev *dev, int mask); > +static int ixgbe_vlan_offload_set(struct rte_eth_dev *dev, int mask); > static void ixgbe_vlan_hw_strip_enable(struct rte_eth_dev *dev, uint16_t queue); > static void ixgbe_vlan_hw_strip_disable(struct rte_eth_dev *dev, uint16_t queue); > static void ixgbe_vlan_hw_extend_enable(struct rte_eth_dev *dev); > @@ -274,7 +274,7 @@ static int ixgbevf_vlan_filter_set(struct rte_eth_dev *dev, > uint16_t vlan_id, int on); > static void ixgbevf_vlan_strip_queue_set(struct rte_eth_dev *dev, > uint16_t queue, int on); > -static void ixgbevf_vlan_offload_set(struct rte_eth_dev *dev, int mask); > +static int ixgbevf_vlan_offload_set(struct rte_eth_dev *dev, int mask); > static void ixgbevf_set_vfta_all(struct rte_eth_dev *dev, bool on); > static int ixgbevf_dev_rx_queue_intr_enable(struct rte_eth_dev *dev, > uint16_t queue_id); > @@ -2125,7 +2125,7 @@ static int eth_ixgbevf_pci_remove(struct rte_pci_device *pci_dev) > */ > } > > -static void > +static int > ixgbe_vlan_offload_set(struct rte_eth_dev *dev, int mask) > { > if (mask & ETH_VLAN_STRIP_MASK) { > @@ -2148,6 +2148,7 @@ static int eth_ixgbevf_pci_remove(struct rte_pci_device *pci_dev) > else > ixgbe_vlan_hw_extend_disable(dev); > } > + return 0; > } > > static void > @@ -2568,9 +2569,13 @@ static int eth_ixgbevf_pci_remove(struct rte_pci_device *pci_dev) > goto error; > } > > - mask = ETH_VLAN_STRIP_MASK | ETH_VLAN_FILTER_MASK | > + mask = ETH_VLAN_STRIP_MASK | ETH_VLAN_FILTER_MASK | > ETH_VLAN_EXTEND_MASK; > - ixgbe_vlan_offload_set(dev, mask); > + err = ixgbe_vlan_offload_set(dev, mask); > + if (err) { > + PMD_INIT_LOG(ERR, "Unable to set VLAN offload"); > + goto error; > + } > > if (dev->data->dev_conf.rxmode.mq_mode == ETH_MQ_RX_VMDQ_ONLY) { > /* Enable vlan filtering for VMDq */ > @@ -4993,7 +4998,12 @@ static int ixgbevf_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev, > /* Set HW strip */ > mask = ETH_VLAN_STRIP_MASK | ETH_VLAN_FILTER_MASK | > ETH_VLAN_EXTEND_MASK; > - ixgbevf_vlan_offload_set(dev, mask); > + err = ixgbevf_vlan_offload_set(dev, mask); > + if (err) { > + PMD_INIT_LOG(ERR, "Unable to set VLAN offload (%d)", err); > + ixgbe_dev_clear_queues(dev); > + return err; > + } > > ixgbevf_dev_rxtx_start(dev); > > @@ -5153,7 +5163,7 @@ static void ixgbevf_set_vfta_all(struct rte_eth_dev *dev, bool on) > ixgbe_vlan_hw_strip_bitmap_set(dev, queue, on); > } > > -static void > +static int > ixgbevf_vlan_offload_set(struct rte_eth_dev *dev, int mask) > { > struct ixgbe_hw *hw = > @@ -5168,6 +5178,7 @@ static void ixgbevf_set_vfta_all(struct rte_eth_dev *dev, bool on) > for (i = 0; i < hw->mac.max_rx_queues; i++) > ixgbevf_vlan_strip_queue_set(dev, i, on); > } > + return 0; > } > > int > diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h > index 43c5384..93e71be 100644 > --- a/drivers/net/mlx5/mlx5.h > +++ b/drivers/net/mlx5/mlx5.h > @@ -283,7 +283,7 @@ int mlx5_xstats_get_names(struct rte_eth_dev *, > /* mlx5_vlan.c */ > > int mlx5_vlan_filter_set(struct rte_eth_dev *, uint16_t, int); > -void mlx5_vlan_offload_set(struct rte_eth_dev *, int); > +int mlx5_vlan_offload_set(struct rte_eth_dev *, int); > void mlx5_vlan_strip_queue_set(struct rte_eth_dev *, uint16_t, int); > > /* mlx5_trigger.c */ > diff --git a/drivers/net/mlx5/mlx5_vlan.c b/drivers/net/mlx5/mlx5_vlan.c > index 1b0fa40..7215909 100644 > --- a/drivers/net/mlx5/mlx5_vlan.c > +++ b/drivers/net/mlx5/mlx5_vlan.c > @@ -210,7 +210,7 @@ > * @param mask > * VLAN offload bit mask. > */ > -void > +int > mlx5_vlan_offload_set(struct rte_eth_dev *dev, int mask) > { > struct priv *priv = dev->data->dev_private; > @@ -230,4 +230,5 @@ > priv_vlan_strip_queue_set(priv, i, hw_vlan_strip); > priv_unlock(priv); > } > + return 0; > } > diff --git a/drivers/net/nfp/nfp_net.c b/drivers/net/nfp/nfp_net.c > index 92b03c4..6473edc 100644 > --- a/drivers/net/nfp/nfp_net.c > +++ b/drivers/net/nfp/nfp_net.c > @@ -2149,11 +2149,12 @@ uint32_t nfp_net_txq_full(struct nfp_net_txq *txq) > return i; > } > > -static void > +static int > nfp_net_vlan_offload_set(struct rte_eth_dev *dev, int mask) > { > uint32_t new_ctrl, update; > struct nfp_net_hw *hw; > + int ret; > > hw = NFP_NET_DEV_PRIVATE_TO_HW(dev->data->dev_private); > new_ctrl = 0; > @@ -2174,14 +2175,14 @@ uint32_t nfp_net_txq_full(struct nfp_net_txq *txq) > new_ctrl = hw->ctrl & ~NFP_NET_CFG_CTRL_RXVLAN; > > if (new_ctrl == 0) > - return; > + return 0; > > update = NFP_NET_CFG_UPDATE_GEN; > > - if (nfp_net_reconfig(hw, new_ctrl, update) < 0) > - return; > - > - hw->ctrl = new_ctrl; > + ret = nfp_net_reconfig(hw, new_ctrl, update); > + if (!ret) > + hw->ctrl = new_ctrl; > + return ret; > } > > /* Update Redirection Table(RETA) of Receive Side Scaling of Ethernet device */ > diff --git a/drivers/net/qede/qede_ethdev.c b/drivers/net/qede/qede_ethdev.c > index 0e05989..644f69d 100644 > --- a/drivers/net/qede/qede_ethdev.c > +++ b/drivers/net/qede/qede_ethdev.c > @@ -975,7 +975,7 @@ static int qede_vlan_filter_set(struct rte_eth_dev *eth_dev, > return rc; > } > > -static void qede_vlan_offload_set(struct rte_eth_dev *eth_dev, int mask) > +static int qede_vlan_offload_set(struct rte_eth_dev *eth_dev, int mask) > { > struct qede_dev *qdev = QEDE_INIT_QDEV(eth_dev); > struct ecore_dev *edev = QEDE_INIT_EDEV(qdev); > @@ -1013,6 +1013,8 @@ static void qede_vlan_offload_set(struct rte_eth_dev *eth_dev, int mask) > > DP_INFO(edev, "vlan offload mask %d vlan-strip %d vlan-filter %d\n", > mask, rxmode->hw_vlan_strip, rxmode->hw_vlan_filter); > + > + return 0; > } > > static void qede_prandom_bytes(uint32_t *buff) > @@ -1157,6 +1159,7 @@ static int qede_dev_configure(struct rte_eth_dev *eth_dev) > struct qede_dev *qdev = QEDE_INIT_QDEV(eth_dev); > struct ecore_dev *edev = QEDE_INIT_EDEV(qdev); > struct rte_eth_rxmode *rxmode = ð_dev->data->dev_conf.rxmode; > + int ret; > > PMD_INIT_FUNC_TRACE(edev); > > @@ -1237,9 +1240,11 @@ static int qede_dev_configure(struct rte_eth_dev *eth_dev) > qdev->enable_lro = rxmode->enable_lro; > > /* Enable VLAN offloads by default */ > - qede_vlan_offload_set(eth_dev, ETH_VLAN_STRIP_MASK | > + ret = qede_vlan_offload_set(eth_dev, ETH_VLAN_STRIP_MASK | > ETH_VLAN_FILTER_MASK | > ETH_VLAN_EXTEND_MASK); > + if (ret) > + return ret; > > DP_INFO(edev, "Device configured with RSS=%d TSS=%d\n", > QEDE_RSS_COUNT(qdev), QEDE_TSS_COUNT(qdev)); > diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c > index e320811..72b4248 100644 > --- a/drivers/net/virtio/virtio_ethdev.c > +++ b/drivers/net/virtio/virtio_ethdev.c > @@ -72,6 +72,7 @@ static void virtio_dev_info_get(struct rte_eth_dev *dev, > struct rte_eth_dev_info *dev_info); > static int virtio_dev_link_update(struct rte_eth_dev *dev, > int wait_to_complete); > +static int virtio_dev_vlan_offload_set(struct rte_eth_dev *dev, int mask); > > static void virtio_set_hwaddr(struct virtio_hw *hw); > static void virtio_get_hwaddr(struct virtio_hw *hw); > @@ -779,6 +780,7 @@ struct rte_virtio_xstats_name_off { > .stats_reset = virtio_dev_stats_reset, > .xstats_reset = virtio_dev_stats_reset, > .link_update = virtio_dev_link_update, > + .vlan_offload_set = virtio_dev_vlan_offload_set, > .rx_queue_setup = virtio_dev_rx_queue_setup, > .rx_queue_intr_enable = virtio_dev_rx_queue_intr_enable, > .rx_queue_intr_disable = virtio_dev_rx_queue_intr_disable, > @@ -1875,6 +1877,25 @@ static void virtio_dev_free_mbufs(struct rte_eth_dev *dev) > return (old.link_status == link.link_status) ? -1 : 0; > } > > +static int > +virtio_dev_vlan_offload_set(struct rte_eth_dev *dev, int mask) > +{ > + const struct rte_eth_rxmode *rxmode = &dev->data->dev_conf.rxmode; > + struct virtio_hw *hw = dev->data->dev_private; > + > + if (rxmode->hw_vlan_filter && > + !vtpci_with_feature(hw, VIRTIO_NET_F_CTRL_VLAN)) { > + PMD_DRV_LOG(NOTICE, > + "vlan filtering not available on this host"); > + return -ENOTSUP; > + } > + > + if (mask & ETH_VLAN_STRIP_MASK) > + hw->vlan_strip = rxmode->hw_vlan_strip; > + > + return 0; > +} > + > static void > virtio_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info) > { > diff --git a/drivers/net/vmxnet3/vmxnet3_ethdev.c b/drivers/net/vmxnet3/vmxnet3_ethdev.c > index 3910991..06735dd 100644 > --- a/drivers/net/vmxnet3/vmxnet3_ethdev.c > +++ b/drivers/net/vmxnet3/vmxnet3_ethdev.c > @@ -100,7 +100,7 @@ static void vmxnet3_dev_info_get(struct rte_eth_dev *dev, > 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 void vmxnet3_dev_vlan_offload_set(struct rte_eth_dev *dev, int mask); > +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, > struct ether_addr *mac_addr); > static void vmxnet3_interrupt_handler(void *param); > @@ -730,8 +730,10 @@ static int eth_vmxnet3_pci_remove(struct rte_pci_device *pci_dev) > devRead->rssConfDesc.confPA = hw->rss_confPA; > } > > - vmxnet3_dev_vlan_offload_set(dev, > + ret = vmxnet3_dev_vlan_offload_set(dev, > ETH_VLAN_STRIP_MASK | ETH_VLAN_FILTER_MASK); > + if (ret) > + return ret; > > vmxnet3_write_mac(hw, dev->data->mac_addrs->addr_bytes); > > @@ -1275,7 +1277,7 @@ static int eth_vmxnet3_pci_remove(struct rte_pci_device *pci_dev) > return 0; > } > > -static void > +static int > vmxnet3_dev_vlan_offload_set(struct rte_eth_dev *dev, int mask) > { > struct vmxnet3_hw *hw = dev->data->dev_private; > @@ -1301,6 +1303,8 @@ static int eth_vmxnet3_pci_remove(struct rte_pci_device *pci_dev) > VMXNET3_WRITE_BAR1_REG(hw, VMXNET3_REG_CMD, > VMXNET3_CMD_UPDATE_VLAN_FILTERS); > } > + > + return 0; > } > > static void > diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c > index 0597641..05e52b8 100644 > --- a/lib/librte_ether/rte_ethdev.c > +++ b/lib/librte_ether/rte_ethdev.c > @@ -2048,29 +2048,35 @@ struct rte_eth_dev * > struct rte_eth_dev *dev; > int ret = 0; > int mask = 0; > - int cur, org = 0; > + int cur, orig = 0; > + uint8_t orig_strip, orig_filter, orig_extend; > > RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV); > dev = &rte_eth_devices[port_id]; > > + /* save original values in case of failure */ > + orig_strip = dev->data->dev_conf.rxmode.hw_vlan_strip; > + orig_filter = dev->data->dev_conf.rxmode.hw_vlan_filter; > + orig_extend = dev->data->dev_conf.rxmode.hw_vlan_extend; > + > /*check which option changed by application*/ > cur = !!(offload_mask & ETH_VLAN_STRIP_OFFLOAD); > - org = !!(dev->data->dev_conf.rxmode.hw_vlan_strip); > - if (cur != org) { > + orig = !!(dev->data->dev_conf.rxmode.hw_vlan_strip); > + if (cur != orig) { > dev->data->dev_conf.rxmode.hw_vlan_strip = (uint8_t)cur; > mask |= ETH_VLAN_STRIP_MASK; > } > > cur = !!(offload_mask & ETH_VLAN_FILTER_OFFLOAD); > - org = !!(dev->data->dev_conf.rxmode.hw_vlan_filter); > - if (cur != org) { > + orig = !!(dev->data->dev_conf.rxmode.hw_vlan_filter); > + if (cur != orig) { > dev->data->dev_conf.rxmode.hw_vlan_filter = (uint8_t)cur; > mask |= ETH_VLAN_FILTER_MASK; > } > > cur = !!(offload_mask & ETH_VLAN_EXTEND_OFFLOAD); > - org = !!(dev->data->dev_conf.rxmode.hw_vlan_extend); > - if (cur != org) { > + orig = !!(dev->data->dev_conf.rxmode.hw_vlan_extend); > + if (cur != orig) { > dev->data->dev_conf.rxmode.hw_vlan_extend = (uint8_t)cur; > mask |= ETH_VLAN_EXTEND_MASK; > } > @@ -2080,7 +2086,13 @@ struct rte_eth_dev * > return ret; > > RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->vlan_offload_set, -ENOTSUP); > - (*dev->dev_ops->vlan_offload_set)(dev, mask); > + ret = (*dev->dev_ops->vlan_offload_set)(dev, mask); > + if (ret) { > + /* hit an error restore original values */ > + dev->data->dev_conf.rxmode.hw_vlan_strip = orig_strip; > + dev->data->dev_conf.rxmode.hw_vlan_filter = orig_filter; > + dev->data->dev_conf.rxmode.hw_vlan_extend = orig_extend; > + } > Currently vlan offload is combining three functions: strip, filter and extend. Not all PMDs in DPDK support all of three. Should not the error we extended to reflect, which of the VLAN offload is not supported? > return ret; > } > diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h > index 0adf327..7254fd0 100644 > --- a/lib/librte_ether/rte_ethdev.h > +++ b/lib/librte_ether/rte_ethdev.h > @@ -1245,7 +1245,7 @@ typedef int (*vlan_tpid_set_t)(struct rte_eth_dev *dev, > enum rte_vlan_type type, uint16_t tpid); > /**< @internal set the outer/inner VLAN-TPID by an Ethernet device. */ > > -typedef void (*vlan_offload_set_t)(struct rte_eth_dev *dev, int mask); > +typedef int (*vlan_offload_set_t)(struct rte_eth_dev *dev, int mask); > /**< @internal set VLAN offload function by an Ethernet device. */ > > typedef int (*vlan_pvid_set_t)(struct rte_eth_dev *dev, > ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [dpdk-dev] [PATCH v4] ethdev: allow returning error on VLAN offload configuration 2017-09-01 7:41 ` Hemant Agrawal @ 2017-09-01 12:54 ` David Harton (dharton) 2017-09-07 9:37 ` Hemant Agrawal 0 siblings, 1 reply; 19+ messages in thread From: David Harton (dharton) @ 2017-09-01 12:54 UTC (permalink / raw) To: Hemant Agrawal, thomas, ferruh.yigit, ajit.khaparde, John Daley (johndale), konstantin.ananyev, jingjing.wu, beilei.xing, jing.d.chen, adrien.mazarguil, nelio.laranjeiro, alejandro.lucero, rasesh.mody, harish.patil, skhare, yliu, maxime.coquelin, allain.legacy Cc: dev > -----Original Message----- > From: Hemant Agrawal [mailto:hemant.agrawal@nxp.com] > Sent: Friday, September 01, 2017 3:41 AM > To: David Harton (dharton) <dharton@cisco.com>; thomas@monjalon.net; > ferruh.yigit@intel.com; ajit.khaparde@broadcom.com; John Daley (johndale) > <johndale@cisco.com>; konstantin.ananyev@intel.com; jingjing.wu@intel.com; > beilei.xing@intel.com; jing.d.chen@intel.com; adrien.mazarguil@6wind.com; > nelio.laranjeiro@6wind.com; alejandro.lucero@netronome.com; > rasesh.mody@cavium.com; harish.patil@cavium.com; skhare@vmware.com; > yliu@fridaylinux.org; maxime.coquelin@redhat.com; > allain.legacy@windriver.com > Cc: dev@dpdk.org > Subject: Re: [PATCH v4] ethdev: allow returning error on VLAN offload > configuration > > On 9/1/2017 8:06 AM, David Harton wrote: > > Some devices may not support or fail setting VLAN offload > > configuration based on dynamic circurmstances so the > > vlan_offload_set_t vector is modified to return an int so the caller > > can determine success or not. > > > > rte_eth_dev_set_vlan_offload is updated to return the value provided > > by the vector when called along with restoring the original offload > > configs on failure. > > > > Existing vlan_offload_set_t vectors are modified to return an int. > > Majority of cases return 0 but a few that actually can fail now return > > their failure codes. > > > > Finally, a vlan_offload_set_t vector is added to virtio to facilitate > > dynamically turning VLAN strip on or off. > > > > Signed-off-by: David Harton <dharton@cisco.com> > > --- > > > > v4 > > * Modified commit message heading > > * Moved rel_note comments from ABI to API section > > * Renamed locals of rte_eth_dev_set_vlan_offload from 'org*' to 'orig*' > > > > v3 > > * Fixed a format error. > > * Apologies...need to figure out why checkpatches.pl keeps saying > > valid patch when I've got soft tabs. > > > > v2 > > * Fixed a missed format error. > > * Removed vlan offload vector call casts and replaced with checks > > for return values. > > > > v1 > > * This is an ABI breakage that has been previously negotiated > > with Thomas and the proposed rel note change is included as well. > > > > doc/guides/rel_notes/release_17_11.rst | 8 +++++++- > > drivers/net/avp/avp_ethdev.c | 12 +++++++++--- > > drivers/net/bnxt/bnxt_ethdev.c | 9 ++++++--- > > drivers/net/dpaa2/dpaa2_ethdev.c | 13 ++++++++++--- > > drivers/net/e1000/em_ethdev.c | 12 +++++++++--- > > drivers/net/e1000/igb_ethdev.c | 12 +++++++++--- > > drivers/net/enic/enic_ethdev.c | 8 +++++--- > > drivers/net/fm10k/fm10k_ethdev.c | 3 ++- > > drivers/net/i40e/i40e_ethdev.c | 11 ++++++++--- > > drivers/net/i40e/i40e_ethdev_vf.c | 9 ++++++--- > > drivers/net/ixgbe/ixgbe_ethdev.c | 25 ++++++++++++++++++------- > > drivers/net/mlx5/mlx5.h | 2 +- > > drivers/net/mlx5/mlx5_vlan.c | 3 ++- > > drivers/net/nfp/nfp_net.c | 13 +++++++------ > > drivers/net/qede/qede_ethdev.c | 9 +++++++-- > > drivers/net/virtio/virtio_ethdev.c | 21 +++++++++++++++++++++ > > drivers/net/vmxnet3/vmxnet3_ethdev.c | 10 +++++++--- > > lib/librte_ether/rte_ethdev.c | 28 ++++++++++++++++++++------- > - > > lib/librte_ether/rte_ethdev.h | 2 +- > > 19 files changed, 155 insertions(+), 55 deletions(-) > > > > diff --git a/doc/guides/rel_notes/release_17_11.rst > > b/doc/guides/rel_notes/release_17_11.rst > > index 170f4f9..22df4fd 100644 > > --- a/doc/guides/rel_notes/release_17_11.rst > > +++ b/doc/guides/rel_notes/release_17_11.rst > > @@ -110,6 +110,13 @@ API Changes > > Also, make sure to start the actual text at the margin. > > ========================================================= > > > > +* **Modified the vlan_offload_set_t function prototype in the ethdev > > +library.** > > + > > + Changed the function prototype of ``vlan_offload_set_t``. The > > + return value has been changed from ``void`` to ``int`` so the > > + caller to knows whether the backing device supports the operation > > + or if the operation was successfully performed. > > + > > > > ABI Changes > > ----------- > > @@ -125,7 +132,6 @@ ABI Changes > > ========================================================= > > > > > > - > > Shared Library Versions > > ----------------------- > > > > diff --git a/drivers/net/avp/avp_ethdev.c > > b/drivers/net/avp/avp_ethdev.c index c746a0e..4011dfa 100644 > > --- a/drivers/net/avp/avp_ethdev.c > > +++ b/drivers/net/avp/avp_ethdev.c > > @@ -70,7 +70,7 @@ static int avp_dev_create(struct rte_pci_device > > *pci_dev, static void avp_dev_close(struct rte_eth_dev *dev); static > > void avp_dev_info_get(struct rte_eth_dev *dev, > > struct rte_eth_dev_info *dev_info); -static void > > avp_vlan_offload_set(struct rte_eth_dev *dev, int mask); > > +static int avp_vlan_offload_set(struct rte_eth_dev *dev, int mask); > > static int avp_dev_link_update(struct rte_eth_dev *dev, int > > wait_to_complete); static void avp_dev_promiscuous_enable(struct > > rte_eth_dev *dev); static void avp_dev_promiscuous_disable(struct > > rte_eth_dev *dev); @@ -2031,7 +2031,12 @@ struct avp_queue { > > mask = (ETH_VLAN_STRIP_MASK | > > ETH_VLAN_FILTER_MASK | > > ETH_VLAN_EXTEND_MASK); > > - avp_vlan_offload_set(eth_dev, mask); > > + ret = avp_vlan_offload_set(eth_dev, mask); > > + if (ret < 0) { > > + PMD_DRV_LOG(ERR, "VLAN offload set failed by host, ret=%d\n", > > + ret); > > + goto unlock; > > + } > > > > /* update device config */ > > memset(&config, 0, sizeof(config)); > > @@ -2214,7 +2219,7 @@ struct avp_queue { > > } > > } > > > > -static void > > +static int > > avp_vlan_offload_set(struct rte_eth_dev *eth_dev, int mask) { > > struct avp_dev *avp = > > AVP_DEV_PRIVATE_TO_HW(eth_dev->data->dev_private); > > @@ -2239,6 +2244,7 @@ struct avp_queue { > > if (eth_dev->data->dev_conf.rxmode.hw_vlan_extend) > > PMD_DRV_LOG(ERR, "VLAN extend offload not supported\n"); > > } > > + return 0; > > } > > > > static void > > diff --git a/drivers/net/bnxt/bnxt_ethdev.c > > b/drivers/net/bnxt/bnxt_ethdev.c index c9d1122..547bd55 100644 > > --- a/drivers/net/bnxt/bnxt_ethdev.c > > +++ b/drivers/net/bnxt/bnxt_ethdev.c > > @@ -144,7 +144,7 @@ > > ETH_RSS_NONFRAG_IPV6_TCP | \ > > ETH_RSS_NONFRAG_IPV6_UDP) > > > > -static void bnxt_vlan_offload_set_op(struct rte_eth_dev *dev, int > > mask); > > +static int bnxt_vlan_offload_set_op(struct rte_eth_dev *dev, int > > +mask); > > > > /***********************/ > > > > @@ -522,7 +522,9 @@ static int bnxt_dev_start_op(struct rte_eth_dev > *eth_dev) > > vlan_mask |= ETH_VLAN_FILTER_MASK; > > if (eth_dev->data->dev_conf.rxmode.hw_vlan_strip) > > vlan_mask |= ETH_VLAN_STRIP_MASK; > > - bnxt_vlan_offload_set_op(eth_dev, vlan_mask); > > + rc = bnxt_vlan_offload_set_op(eth_dev, vlan_mask); > > + if (rc) > > + goto error; > > > > return 0; > > > > @@ -1275,7 +1277,7 @@ static int bnxt_vlan_filter_set_op(struct > rte_eth_dev *eth_dev, > > return bnxt_del_vlan_filter(bp, vlan_id); } > > > > -static void > > +static int > > bnxt_vlan_offload_set_op(struct rte_eth_dev *dev, int mask) { > > struct bnxt *bp = (struct bnxt *)dev->data->dev_private; @@ -1307,6 > > +1309,7 @@ static int bnxt_vlan_filter_set_op(struct rte_eth_dev > > *eth_dev, > > > > if (mask & ETH_VLAN_EXTEND_MASK) > > RTE_LOG(ERR, PMD, "Extend VLAN Not supported\n"); > > + return 0; > > } > > > > static void > > diff --git a/drivers/net/dpaa2/dpaa2_ethdev.c > > b/drivers/net/dpaa2/dpaa2_ethdev.c > > index 429b3a0..3390cb3 100644 > > --- a/drivers/net/dpaa2/dpaa2_ethdev.c > > +++ b/drivers/net/dpaa2/dpaa2_ethdev.c > > @@ -138,7 +138,7 @@ > > return ret; > > } > > > > -static void > > +static int > > dpaa2_vlan_offload_set(struct rte_eth_dev *dev, int mask) { > > struct dpaa2_dev_priv *priv = dev->data->dev_private; @@ -158,6 > > +158,7 @@ > > RTE_LOG(ERR, PMD, "Unable to set vlan filter = %d\n", > > ret); > > } > > + return 0; > > } > > > > static int > > @@ -643,8 +644,14 @@ > > return ret; > > } > > /* VLAN Offload Settings */ > > - if (priv->max_vlan_filters) > > - dpaa2_vlan_offload_set(dev, ETH_VLAN_FILTER_MASK); > > + if (priv->max_vlan_filters) { > > + ret = dpaa2_vlan_offload_set(dev, ETH_VLAN_FILTER_MASK); > > + if (ret) { > > + PMD_INIT_LOG(ERR, "Error to dpaa2_vlan_offload_set:" > > + "code = %d\n", ret); > > + return ret; > > + } > > + } > > > > return 0; > > } > > diff --git a/drivers/net/e1000/em_ethdev.c > > b/drivers/net/e1000/em_ethdev.c index 3d4ab93..51f49d8 100644 > > --- a/drivers/net/e1000/em_ethdev.c > > +++ b/drivers/net/e1000/em_ethdev.c > > @@ -99,7 +99,7 @@ static int eth_em_interrupt_action(struct > > rte_eth_dev *dev, > > > > static int eth_em_vlan_filter_set(struct rte_eth_dev *dev, > > uint16_t vlan_id, int on); > > -static void eth_em_vlan_offload_set(struct rte_eth_dev *dev, int > > mask); > > +static int eth_em_vlan_offload_set(struct rte_eth_dev *dev, int > > +mask); > > static void em_vlan_hw_filter_enable(struct rte_eth_dev *dev); > > static void em_vlan_hw_filter_disable(struct rte_eth_dev *dev); > > static void em_vlan_hw_strip_enable(struct rte_eth_dev *dev); @@ > > -668,7 +668,12 @@ static int eth_em_pci_remove(struct rte_pci_device > > *pci_dev) > > > > mask = ETH_VLAN_STRIP_MASK | ETH_VLAN_FILTER_MASK | \ > > ETH_VLAN_EXTEND_MASK; > > - eth_em_vlan_offload_set(dev, mask); > > + ret = eth_em_vlan_offload_set(dev, mask); > > + if (ret) { > > + PMD_INIT_LOG(ERR, "Unable to update vlan offload"); > > + em_dev_clear_queues(dev); > > + return ret; > > + } > > > > /* Set Interrupt Throttling Rate to maximum allowed value. */ > > E1000_WRITE_REG(hw, E1000_ITR, UINT16_MAX); @@ -1447,7 +1452,7 @@ > > static int eth_em_pci_remove(struct rte_pci_device *pci_dev) > > E1000_WRITE_REG(hw, E1000_CTRL, reg); } > > > > -static void > > +static int > > eth_em_vlan_offload_set(struct rte_eth_dev *dev, int mask) { > > if(mask & ETH_VLAN_STRIP_MASK){ > > @@ -1463,6 +1468,7 @@ static int eth_em_pci_remove(struct rte_pci_device > *pci_dev) > > else > > em_vlan_hw_filter_disable(dev); > > } > > + return 0; > > } > > > > /* > > diff --git a/drivers/net/e1000/igb_ethdev.c > > b/drivers/net/e1000/igb_ethdev.c index e4f7a9f..fa15ee9 100644 > > --- a/drivers/net/e1000/igb_ethdev.c > > +++ b/drivers/net/e1000/igb_ethdev.c > > @@ -157,7 +157,7 @@ static int eth_igb_vlan_filter_set(struct > > rte_eth_dev *dev, static int eth_igb_vlan_tpid_set(struct rte_eth_dev > *dev, > > enum rte_vlan_type vlan_type, > > uint16_t tpid_id); > > -static void eth_igb_vlan_offload_set(struct rte_eth_dev *dev, int > > mask); > > +static int eth_igb_vlan_offload_set(struct rte_eth_dev *dev, int > > +mask); > > > > static void igb_vlan_hw_filter_enable(struct rte_eth_dev *dev); > > static void igb_vlan_hw_filter_disable(struct rte_eth_dev *dev); @@ > > -1400,7 +1400,12 @@ static int eth_igbvf_pci_remove(struct > rte_pci_device *pci_dev) > > */ > > mask = ETH_VLAN_STRIP_MASK | ETH_VLAN_FILTER_MASK | \ > > ETH_VLAN_EXTEND_MASK; > > - eth_igb_vlan_offload_set(dev, mask); > > + ret = eth_igb_vlan_offload_set(dev, mask); > > + if (ret) { > > + PMD_INIT_LOG(ERR, "Unable to set vlan offload"); > > + igb_dev_clear_queues(dev); > > + return ret; > > + } > > > > if (dev->data->dev_conf.rxmode.mq_mode == ETH_MQ_RX_VMDQ_ONLY) { > > /* Enable VLAN filter since VMDq always use VLAN filter */ @@ > > -2715,7 +2720,7 @@ static int eth_igbvf_xstats_get_names(__rte_unused > struct rte_eth_dev *dev, > > 2 * VLAN_TAG_SIZE); > > } > > > > -static void > > +static int > > eth_igb_vlan_offload_set(struct rte_eth_dev *dev, int mask) { > > if(mask & ETH_VLAN_STRIP_MASK){ > > @@ -2738,6 +2743,7 @@ static int eth_igbvf_xstats_get_names(__rte_unused > struct rte_eth_dev *dev, > > else > > igb_vlan_hw_extend_disable(dev); > > } > > + return 0; > > } > > > > > > diff --git a/drivers/net/enic/enic_ethdev.c > > b/drivers/net/enic/enic_ethdev.c index da8fec2..fc1eac2 100644 > > --- a/drivers/net/enic/enic_ethdev.c > > +++ b/drivers/net/enic/enic_ethdev.c > > @@ -347,7 +347,7 @@ static int enicpmd_vlan_filter_set(struct > rte_eth_dev *eth_dev, > > return err; > > } > > > > -static void enicpmd_vlan_offload_set(struct rte_eth_dev *eth_dev, int > > mask) > > +static int enicpmd_vlan_offload_set(struct rte_eth_dev *eth_dev, int > > +mask) > > { > > struct enic *enic = pmd_priv(eth_dev); > > > > @@ -371,6 +371,8 @@ static void enicpmd_vlan_offload_set(struct > rte_eth_dev *eth_dev, int mask) > > dev_warning(enic, > > "Configuration of extended VLAN is not supported\n"); > > } > > + > > + return 0; > > } > > > > static int enicpmd_dev_configure(struct rte_eth_dev *eth_dev) @@ > > -392,9 +394,9 @@ static int enicpmd_dev_configure(struct rte_eth_dev > *eth_dev) > > eth_dev->data->dev_conf.rxmode.split_hdr_size); > > } > > > > - enicpmd_vlan_offload_set(eth_dev, ETH_VLAN_STRIP_MASK); > > + ret = enicpmd_vlan_offload_set(eth_dev, ETH_VLAN_STRIP_MASK); > > enic->hw_ip_checksum = eth_dev->data- > >dev_conf.rxmode.hw_ip_checksum; > > - return 0; > > + return ret; > > } > > > > /* Start the device. > > diff --git a/drivers/net/fm10k/fm10k_ethdev.c > > b/drivers/net/fm10k/fm10k_ethdev.c > > index e60d3a3..f4626f7 100644 > > --- a/drivers/net/fm10k/fm10k_ethdev.c > > +++ b/drivers/net/fm10k/fm10k_ethdev.c > > @@ -1590,7 +1590,7 @@ static int fm10k_xstats_get_names(__rte_unused > struct rte_eth_dev *dev, > > return 0; > > } > > > > -static void > > +static int > > fm10k_vlan_offload_set(struct rte_eth_dev *dev, int mask) { > > if (mask & ETH_VLAN_STRIP_MASK) { > > @@ -1609,6 +1609,7 @@ static int fm10k_xstats_get_names(__rte_unused > struct rte_eth_dev *dev, > > if (!dev->data->dev_conf.rxmode.hw_vlan_filter) > > PMD_INIT_LOG(ERR, "VLAN filter is always on in fm10k"); > > } > > + return 0; > > } > > > > /* Add/Remove a MAC address, and update filters to main VSI */ diff > > --git a/drivers/net/i40e/i40e_ethdev.c > > b/drivers/net/i40e/i40e_ethdev.c index 00b6082..d03a44b 100644 > > --- a/drivers/net/i40e/i40e_ethdev.c > > +++ b/drivers/net/i40e/i40e_ethdev.c > > @@ -278,7 +278,7 @@ static int i40e_vlan_filter_set(struct rte_eth_dev > > *dev, static int i40e_vlan_tpid_set(struct rte_eth_dev *dev, > > enum rte_vlan_type vlan_type, > > uint16_t tpid); > > -static void i40e_vlan_offload_set(struct rte_eth_dev *dev, int mask); > > +static int i40e_vlan_offload_set(struct rte_eth_dev *dev, int mask); > > static void i40e_vlan_strip_queue_set(struct rte_eth_dev *dev, > > uint16_t queue, > > int on); > > @@ -3130,7 +3130,7 @@ static int i40e_dev_xstats_get_names(__rte_unused > struct rte_eth_dev *dev, > > return ret; > > } > > > > -static void > > +static int > > i40e_vlan_offload_set(struct rte_eth_dev *dev, int mask) { > > struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private); > > @@ -3163,6 +3163,7 @@ static int i40e_dev_xstats_get_names(__rte_unused > struct rte_eth_dev *dev, > > else > > i40e_vsi_config_double_vlan(vsi, FALSE); > > } > > + return 0; > > } > > > > static void > > @@ -5216,7 +5217,11 @@ struct i40e_vsi * > > > > /* Apply vlan offload setting */ > > mask = ETH_VLAN_STRIP_MASK | ETH_VLAN_FILTER_MASK; > > - i40e_vlan_offload_set(dev, mask); > > + ret = i40e_vlan_offload_set(dev, mask); > > + if (ret) { > > + PMD_DRV_LOG(INFO, "Failed to update vlan offload"); > > + return ret; > > + } > > > > /* Apply double-vlan setting, not implemented yet */ > > > > diff --git a/drivers/net/i40e/i40e_ethdev_vf.c > > b/drivers/net/i40e/i40e_ethdev_vf.c > > index f6d8293..f7fffc2 100644 > > --- a/drivers/net/i40e/i40e_ethdev_vf.c > > +++ b/drivers/net/i40e/i40e_ethdev_vf.c > > @@ -118,7 +118,7 @@ static int i40evf_dev_xstats_get_names(struct > > rte_eth_dev *dev, static void i40evf_dev_xstats_reset(struct > > rte_eth_dev *dev); static int i40evf_vlan_filter_set(struct rte_eth_dev > *dev, > > uint16_t vlan_id, int on); > > -static void i40evf_vlan_offload_set(struct rte_eth_dev *dev, int > > mask); > > +static int i40evf_vlan_offload_set(struct rte_eth_dev *dev, int > > +mask); > > static int i40evf_vlan_pvid_set(struct rte_eth_dev *dev, uint16_t pvid, > > int on); > > static void i40evf_dev_close(struct rte_eth_dev *dev); @@ -1634,7 > > +1634,9 @@ static int eth_i40evf_pci_remove(struct rte_pci_device > *pci_dev) > > int ret; > > > > /* Apply vlan offload setting */ > > - i40evf_vlan_offload_set(dev, ETH_VLAN_STRIP_MASK); > > + ret = i40evf_vlan_offload_set(dev, ETH_VLAN_STRIP_MASK); > > + if (ret) > > + return ret; > > > > /* Apply pvid setting */ > > ret = i40evf_vlan_pvid_set(dev, data->dev_conf.txmode.pvid, @@ > > -1642,7 +1644,7 @@ static int eth_i40evf_pci_remove(struct > rte_pci_device *pci_dev) > > return ret; > > } > > > > -static void > > +static int > > i40evf_vlan_offload_set(struct rte_eth_dev *dev, int mask) { > > struct rte_eth_conf *dev_conf = &dev->data->dev_conf; @@ -1655,6 > > +1657,7 @@ static int eth_i40evf_pci_remove(struct rte_pci_device > *pci_dev) > > else > > i40evf_disable_vlan_strip(dev); > > } > > + return 0; > > } > > > > static int > > diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c > > b/drivers/net/ixgbe/ixgbe_ethdev.c > > index 22171d8..1ec5aaf 100644 > > --- a/drivers/net/ixgbe/ixgbe_ethdev.c > > +++ b/drivers/net/ixgbe/ixgbe_ethdev.c > > @@ -218,7 +218,7 @@ static void ixgbe_vlan_hw_strip_bitmap_set(struct > rte_eth_dev *dev, > > uint16_t queue, bool on); > > static void ixgbe_vlan_strip_queue_set(struct rte_eth_dev *dev, > uint16_t queue, > > int on); > > -static void ixgbe_vlan_offload_set(struct rte_eth_dev *dev, int > > mask); > > +static int ixgbe_vlan_offload_set(struct rte_eth_dev *dev, int mask); > > static void ixgbe_vlan_hw_strip_enable(struct rte_eth_dev *dev, > > uint16_t queue); static void ixgbe_vlan_hw_strip_disable(struct > > rte_eth_dev *dev, uint16_t queue); static void > > ixgbe_vlan_hw_extend_enable(struct rte_eth_dev *dev); @@ -274,7 +274,7 > @@ static int ixgbevf_vlan_filter_set(struct rte_eth_dev *dev, > > uint16_t vlan_id, int on); > > static void ixgbevf_vlan_strip_queue_set(struct rte_eth_dev *dev, > > uint16_t queue, int on); > > -static void ixgbevf_vlan_offload_set(struct rte_eth_dev *dev, int > > mask); > > +static int ixgbevf_vlan_offload_set(struct rte_eth_dev *dev, int > > +mask); > > static void ixgbevf_set_vfta_all(struct rte_eth_dev *dev, bool on); > > static int ixgbevf_dev_rx_queue_intr_enable(struct rte_eth_dev *dev, > > uint16_t queue_id); > > @@ -2125,7 +2125,7 @@ static int eth_ixgbevf_pci_remove(struct > rte_pci_device *pci_dev) > > */ > > } > > > > -static void > > +static int > > ixgbe_vlan_offload_set(struct rte_eth_dev *dev, int mask) { > > if (mask & ETH_VLAN_STRIP_MASK) { > > @@ -2148,6 +2148,7 @@ static int eth_ixgbevf_pci_remove(struct > rte_pci_device *pci_dev) > > else > > ixgbe_vlan_hw_extend_disable(dev); > > } > > + return 0; > > } > > > > static void > > @@ -2568,9 +2569,13 @@ static int eth_ixgbevf_pci_remove(struct > rte_pci_device *pci_dev) > > goto error; > > } > > > > - mask = ETH_VLAN_STRIP_MASK | ETH_VLAN_FILTER_MASK | > > + mask = ETH_VLAN_STRIP_MASK | ETH_VLAN_FILTER_MASK | > > ETH_VLAN_EXTEND_MASK; > > - ixgbe_vlan_offload_set(dev, mask); > > + err = ixgbe_vlan_offload_set(dev, mask); > > + if (err) { > > + PMD_INIT_LOG(ERR, "Unable to set VLAN offload"); > > + goto error; > > + } > > > > if (dev->data->dev_conf.rxmode.mq_mode == ETH_MQ_RX_VMDQ_ONLY) { > > /* Enable vlan filtering for VMDq */ @@ -4993,7 +4998,12 @@ > static > > int ixgbevf_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev, > > /* Set HW strip */ > > mask = ETH_VLAN_STRIP_MASK | ETH_VLAN_FILTER_MASK | > > ETH_VLAN_EXTEND_MASK; > > - ixgbevf_vlan_offload_set(dev, mask); > > + err = ixgbevf_vlan_offload_set(dev, mask); > > + if (err) { > > + PMD_INIT_LOG(ERR, "Unable to set VLAN offload (%d)", err); > > + ixgbe_dev_clear_queues(dev); > > + return err; > > + } > > > > ixgbevf_dev_rxtx_start(dev); > > > > @@ -5153,7 +5163,7 @@ static void ixgbevf_set_vfta_all(struct > rte_eth_dev *dev, bool on) > > ixgbe_vlan_hw_strip_bitmap_set(dev, queue, on); } > > > > -static void > > +static int > > ixgbevf_vlan_offload_set(struct rte_eth_dev *dev, int mask) { > > struct ixgbe_hw *hw = > > @@ -5168,6 +5178,7 @@ static void ixgbevf_set_vfta_all(struct > rte_eth_dev *dev, bool on) > > for (i = 0; i < hw->mac.max_rx_queues; i++) > > ixgbevf_vlan_strip_queue_set(dev, i, on); > > } > > + return 0; > > } > > > > int > > diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h index > > 43c5384..93e71be 100644 > > --- a/drivers/net/mlx5/mlx5.h > > +++ b/drivers/net/mlx5/mlx5.h > > @@ -283,7 +283,7 @@ int mlx5_xstats_get_names(struct rte_eth_dev *, > > /* mlx5_vlan.c */ > > > > int mlx5_vlan_filter_set(struct rte_eth_dev *, uint16_t, int); -void > > mlx5_vlan_offload_set(struct rte_eth_dev *, int); > > +int mlx5_vlan_offload_set(struct rte_eth_dev *, int); > > void mlx5_vlan_strip_queue_set(struct rte_eth_dev *, uint16_t, int); > > > > /* mlx5_trigger.c */ > > diff --git a/drivers/net/mlx5/mlx5_vlan.c > > b/drivers/net/mlx5/mlx5_vlan.c index 1b0fa40..7215909 100644 > > --- a/drivers/net/mlx5/mlx5_vlan.c > > +++ b/drivers/net/mlx5/mlx5_vlan.c > > @@ -210,7 +210,7 @@ > > * @param mask > > * VLAN offload bit mask. > > */ > > -void > > +int > > mlx5_vlan_offload_set(struct rte_eth_dev *dev, int mask) { > > struct priv *priv = dev->data->dev_private; @@ -230,4 +230,5 @@ > > priv_vlan_strip_queue_set(priv, i, hw_vlan_strip); > > priv_unlock(priv); > > } > > + return 0; > > } > > diff --git a/drivers/net/nfp/nfp_net.c b/drivers/net/nfp/nfp_net.c > > index 92b03c4..6473edc 100644 > > --- a/drivers/net/nfp/nfp_net.c > > +++ b/drivers/net/nfp/nfp_net.c > > @@ -2149,11 +2149,12 @@ uint32_t nfp_net_txq_full(struct nfp_net_txq > *txq) > > return i; > > } > > > > -static void > > +static int > > nfp_net_vlan_offload_set(struct rte_eth_dev *dev, int mask) { > > uint32_t new_ctrl, update; > > struct nfp_net_hw *hw; > > + int ret; > > > > hw = NFP_NET_DEV_PRIVATE_TO_HW(dev->data->dev_private); > > new_ctrl = 0; > > @@ -2174,14 +2175,14 @@ uint32_t nfp_net_txq_full(struct nfp_net_txq > *txq) > > new_ctrl = hw->ctrl & ~NFP_NET_CFG_CTRL_RXVLAN; > > > > if (new_ctrl == 0) > > - return; > > + return 0; > > > > update = NFP_NET_CFG_UPDATE_GEN; > > > > - if (nfp_net_reconfig(hw, new_ctrl, update) < 0) > > - return; > > - > > - hw->ctrl = new_ctrl; > > + ret = nfp_net_reconfig(hw, new_ctrl, update); > > + if (!ret) > > + hw->ctrl = new_ctrl; > > + return ret; > > } > > > > /* Update Redirection Table(RETA) of Receive Side Scaling of Ethernet > device */ > > diff --git a/drivers/net/qede/qede_ethdev.c > b/drivers/net/qede/qede_ethdev.c > > index 0e05989..644f69d 100644 > > --- a/drivers/net/qede/qede_ethdev.c > > +++ b/drivers/net/qede/qede_ethdev.c > > @@ -975,7 +975,7 @@ static int qede_vlan_filter_set(struct rte_eth_dev > *eth_dev, > > return rc; > > } > > > > -static void qede_vlan_offload_set(struct rte_eth_dev *eth_dev, int > mask) > > +static int qede_vlan_offload_set(struct rte_eth_dev *eth_dev, int mask) > > { > > struct qede_dev *qdev = QEDE_INIT_QDEV(eth_dev); > > struct ecore_dev *edev = QEDE_INIT_EDEV(qdev); > > @@ -1013,6 +1013,8 @@ static void qede_vlan_offload_set(struct > rte_eth_dev *eth_dev, int mask) > > > > DP_INFO(edev, "vlan offload mask %d vlan-strip %d vlan-filter %d\n", > > mask, rxmode->hw_vlan_strip, rxmode->hw_vlan_filter); > > + > > + return 0; > > } > > > > static void qede_prandom_bytes(uint32_t *buff) > > @@ -1157,6 +1159,7 @@ static int qede_dev_configure(struct rte_eth_dev > *eth_dev) > > struct qede_dev *qdev = QEDE_INIT_QDEV(eth_dev); > > struct ecore_dev *edev = QEDE_INIT_EDEV(qdev); > > struct rte_eth_rxmode *rxmode = ð_dev->data->dev_conf.rxmode; > > + int ret; > > > > PMD_INIT_FUNC_TRACE(edev); > > > > @@ -1237,9 +1240,11 @@ static int qede_dev_configure(struct rte_eth_dev > *eth_dev) > > qdev->enable_lro = rxmode->enable_lro; > > > > /* Enable VLAN offloads by default */ > > - qede_vlan_offload_set(eth_dev, ETH_VLAN_STRIP_MASK | > > + ret = qede_vlan_offload_set(eth_dev, ETH_VLAN_STRIP_MASK | > > ETH_VLAN_FILTER_MASK | > > ETH_VLAN_EXTEND_MASK); > > + if (ret) > > + return ret; > > > > DP_INFO(edev, "Device configured with RSS=%d TSS=%d\n", > > QEDE_RSS_COUNT(qdev), QEDE_TSS_COUNT(qdev)); > > diff --git a/drivers/net/virtio/virtio_ethdev.c > b/drivers/net/virtio/virtio_ethdev.c > > index e320811..72b4248 100644 > > --- a/drivers/net/virtio/virtio_ethdev.c > > +++ b/drivers/net/virtio/virtio_ethdev.c > > @@ -72,6 +72,7 @@ static void virtio_dev_info_get(struct rte_eth_dev > *dev, > > struct rte_eth_dev_info *dev_info); > > static int virtio_dev_link_update(struct rte_eth_dev *dev, > > int wait_to_complete); > > +static int virtio_dev_vlan_offload_set(struct rte_eth_dev *dev, int > mask); > > > > static void virtio_set_hwaddr(struct virtio_hw *hw); > > static void virtio_get_hwaddr(struct virtio_hw *hw); > > @@ -779,6 +780,7 @@ struct rte_virtio_xstats_name_off { > > .stats_reset = virtio_dev_stats_reset, > > .xstats_reset = virtio_dev_stats_reset, > > .link_update = virtio_dev_link_update, > > + .vlan_offload_set = virtio_dev_vlan_offload_set, > > .rx_queue_setup = virtio_dev_rx_queue_setup, > > .rx_queue_intr_enable = virtio_dev_rx_queue_intr_enable, > > .rx_queue_intr_disable = virtio_dev_rx_queue_intr_disable, > > @@ -1875,6 +1877,25 @@ static void virtio_dev_free_mbufs(struct > rte_eth_dev *dev) > > return (old.link_status == link.link_status) ? -1 : 0; > > } > > > > +static int > > +virtio_dev_vlan_offload_set(struct rte_eth_dev *dev, int mask) > > +{ > > + const struct rte_eth_rxmode *rxmode = &dev->data->dev_conf.rxmode; > > + struct virtio_hw *hw = dev->data->dev_private; > > + > > + if (rxmode->hw_vlan_filter && > > + !vtpci_with_feature(hw, VIRTIO_NET_F_CTRL_VLAN)) { > > + PMD_DRV_LOG(NOTICE, > > + "vlan filtering not available on this host"); > > + return -ENOTSUP; > > + } > > + > > + if (mask & ETH_VLAN_STRIP_MASK) > > + hw->vlan_strip = rxmode->hw_vlan_strip; > > + > > + return 0; > > +} > > + > > static void > > virtio_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info > *dev_info) > > { > > diff --git a/drivers/net/vmxnet3/vmxnet3_ethdev.c > b/drivers/net/vmxnet3/vmxnet3_ethdev.c > > index 3910991..06735dd 100644 > > --- a/drivers/net/vmxnet3/vmxnet3_ethdev.c > > +++ b/drivers/net/vmxnet3/vmxnet3_ethdev.c > > @@ -100,7 +100,7 @@ static void vmxnet3_dev_info_get(struct rte_eth_dev > *dev, > > 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 void vmxnet3_dev_vlan_offload_set(struct rte_eth_dev *dev, int > mask); > > +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, > > struct ether_addr *mac_addr); > > static void vmxnet3_interrupt_handler(void *param); > > @@ -730,8 +730,10 @@ static int eth_vmxnet3_pci_remove(struct > rte_pci_device *pci_dev) > > devRead->rssConfDesc.confPA = hw->rss_confPA; > > } > > > > - vmxnet3_dev_vlan_offload_set(dev, > > + ret = vmxnet3_dev_vlan_offload_set(dev, > > ETH_VLAN_STRIP_MASK | ETH_VLAN_FILTER_MASK); > > + if (ret) > > + return ret; > > > > vmxnet3_write_mac(hw, dev->data->mac_addrs->addr_bytes); > > > > @@ -1275,7 +1277,7 @@ static int eth_vmxnet3_pci_remove(struct > rte_pci_device *pci_dev) > > return 0; > > } > > > > -static void > > +static int > > vmxnet3_dev_vlan_offload_set(struct rte_eth_dev *dev, int mask) > > { > > struct vmxnet3_hw *hw = dev->data->dev_private; > > @@ -1301,6 +1303,8 @@ static int eth_vmxnet3_pci_remove(struct > rte_pci_device *pci_dev) > > VMXNET3_WRITE_BAR1_REG(hw, VMXNET3_REG_CMD, > > VMXNET3_CMD_UPDATE_VLAN_FILTERS); > > } > > + > > + return 0; > > } > > > > static void > > diff --git a/lib/librte_ether/rte_ethdev.c > b/lib/librte_ether/rte_ethdev.c > > index 0597641..05e52b8 100644 > > --- a/lib/librte_ether/rte_ethdev.c > > +++ b/lib/librte_ether/rte_ethdev.c > > @@ -2048,29 +2048,35 @@ struct rte_eth_dev * > > struct rte_eth_dev *dev; > > int ret = 0; > > int mask = 0; > > - int cur, org = 0; > > + int cur, orig = 0; > > + uint8_t orig_strip, orig_filter, orig_extend; > > > > RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV); > > dev = &rte_eth_devices[port_id]; > > > > + /* save original values in case of failure */ > > + orig_strip = dev->data->dev_conf.rxmode.hw_vlan_strip; > > + orig_filter = dev->data->dev_conf.rxmode.hw_vlan_filter; > > + orig_extend = dev->data->dev_conf.rxmode.hw_vlan_extend; > > + > > /*check which option changed by application*/ > > cur = !!(offload_mask & ETH_VLAN_STRIP_OFFLOAD); > > - org = !!(dev->data->dev_conf.rxmode.hw_vlan_strip); > > - if (cur != org) { > > + orig = !!(dev->data->dev_conf.rxmode.hw_vlan_strip); > > + if (cur != orig) { > > dev->data->dev_conf.rxmode.hw_vlan_strip = (uint8_t)cur; > > mask |= ETH_VLAN_STRIP_MASK; > > } > > > > cur = !!(offload_mask & ETH_VLAN_FILTER_OFFLOAD); > > - org = !!(dev->data->dev_conf.rxmode.hw_vlan_filter); > > - if (cur != org) { > > + orig = !!(dev->data->dev_conf.rxmode.hw_vlan_filter); > > + if (cur != orig) { > > dev->data->dev_conf.rxmode.hw_vlan_filter = (uint8_t)cur; > > mask |= ETH_VLAN_FILTER_MASK; > > } > > > > cur = !!(offload_mask & ETH_VLAN_EXTEND_OFFLOAD); > > - org = !!(dev->data->dev_conf.rxmode.hw_vlan_extend); > > - if (cur != org) { > > + orig = !!(dev->data->dev_conf.rxmode.hw_vlan_extend); > > + if (cur != orig) { > > dev->data->dev_conf.rxmode.hw_vlan_extend = (uint8_t)cur; > > mask |= ETH_VLAN_EXTEND_MASK; > > } > > @@ -2080,7 +2086,13 @@ struct rte_eth_dev * > > return ret; > > > > RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->vlan_offload_set, -ENOTSUP); > > - (*dev->dev_ops->vlan_offload_set)(dev, mask); > > + ret = (*dev->dev_ops->vlan_offload_set)(dev, mask); > > + if (ret) { > > + /* hit an error restore original values */ > > + dev->data->dev_conf.rxmode.hw_vlan_strip = orig_strip; > > + dev->data->dev_conf.rxmode.hw_vlan_filter = orig_filter; > > + dev->data->dev_conf.rxmode.hw_vlan_extend = orig_extend; > > + } > > > Currently vlan offload is combining three functions: > strip, filter and extend. > > Not all PMDs in DPDK support all of three. > Should not the error we extended to reflect, which of the VLAN offload > is not supported? There are many examples of APIs that not all PMDs support. There are also other APIs that manipulate many attributes but do not communicate which attribute fails on set. Solving these issues I believe it outside the scope of this change and it requires a larger community discussion. This proposed change at least adheres to the existing paradigm. > > > return ret; > > } > > diff --git a/lib/librte_ether/rte_ethdev.h > b/lib/librte_ether/rte_ethdev.h > > index 0adf327..7254fd0 100644 > > --- a/lib/librte_ether/rte_ethdev.h > > +++ b/lib/librte_ether/rte_ethdev.h > > @@ -1245,7 +1245,7 @@ typedef int (*vlan_tpid_set_t)(struct rte_eth_dev > *dev, > > enum rte_vlan_type type, uint16_t tpid); > > /**< @internal set the outer/inner VLAN-TPID by an Ethernet device. */ > > > > -typedef void (*vlan_offload_set_t)(struct rte_eth_dev *dev, int mask); > > +typedef int (*vlan_offload_set_t)(struct rte_eth_dev *dev, int mask); > > /**< @internal set VLAN offload function by an Ethernet device. */ > > > > typedef int (*vlan_pvid_set_t)(struct rte_eth_dev *dev, > > ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [dpdk-dev] [PATCH v4] ethdev: allow returning error on VLAN offload configuration 2017-09-01 12:54 ` David Harton (dharton) @ 2017-09-07 9:37 ` Hemant Agrawal 2017-09-07 12:09 ` David Harton (dharton) 0 siblings, 1 reply; 19+ messages in thread From: Hemant Agrawal @ 2017-09-07 9:37 UTC (permalink / raw) To: David Harton (dharton), thomas, ferruh.yigit, ajit.khaparde, John Daley (johndale), konstantin.ananyev, jingjing.wu, beilei.xing, jing.d.chen, adrien.mazarguil, nelio.laranjeiro, alejandro.lucero, rasesh.mody, harish.patil, skhare, yliu, maxime.coquelin@redhat.com Cc: dev On 9/1/2017 6:24 PM, David Harton (dharton) wrote: > > >> -----Original Message----- >> From: Hemant Agrawal [mailto:hemant.agrawal@nxp.com] >> Sent: Friday, September 01, 2017 3:41 AM >> Subject: Re: [PATCH v4] ethdev: allow returning error on VLAN offload >> configuration >> >> On 9/1/2017 8:06 AM, David Harton wrote: >>> Some devices may not support or fail setting VLAN offload >>> configuration based on dynamic circurmstances so the >>> vlan_offload_set_t vector is modified to return an int so the caller >>> can determine success or not. >>> >>> rte_eth_dev_set_vlan_offload is updated to return the value provided >>> by the vector when called along with restoring the original offload >>> configs on failure. >>> >>> Existing vlan_offload_set_t vectors are modified to return an int. >>> Majority of cases return 0 but a few that actually can fail now return >>> their failure codes. >>> >>> Finally, a vlan_offload_set_t vector is added to virtio to facilitate >>> dynamically turning VLAN strip on or off. >>> >>> Signed-off-by: David Harton <dharton@cisco.com> >>> --- <snip>.... >>> diff --git a/lib/librte_ether/rte_ethdev.c >> b/lib/librte_ether/rte_ethdev.c >>> index 0597641..05e52b8 100644 >>> --- a/lib/librte_ether/rte_ethdev.c >>> +++ b/lib/librte_ether/rte_ethdev.c >>> @@ -2048,29 +2048,35 @@ struct rte_eth_dev * >>> struct rte_eth_dev *dev; >>> int ret = 0; >>> int mask = 0; >>> - int cur, org = 0; >>> + int cur, orig = 0; >>> + uint8_t orig_strip, orig_filter, orig_extend; >>> >>> RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV); >>> dev = &rte_eth_devices[port_id]; >>> >>> + /* save original values in case of failure */ >>> + orig_strip = dev->data->dev_conf.rxmode.hw_vlan_strip; >>> + orig_filter = dev->data->dev_conf.rxmode.hw_vlan_filter; >>> + orig_extend = dev->data->dev_conf.rxmode.hw_vlan_extend; >>> + >>> /*check which option changed by application*/ >>> cur = !!(offload_mask & ETH_VLAN_STRIP_OFFLOAD); >>> - org = !!(dev->data->dev_conf.rxmode.hw_vlan_strip); >>> - if (cur != org) { >>> + orig = !!(dev->data->dev_conf.rxmode.hw_vlan_strip); >>> + if (cur != orig) { >>> dev->data->dev_conf.rxmode.hw_vlan_strip = (uint8_t)cur; >>> mask |= ETH_VLAN_STRIP_MASK; >>> } >>> >>> cur = !!(offload_mask & ETH_VLAN_FILTER_OFFLOAD); >>> - org = !!(dev->data->dev_conf.rxmode.hw_vlan_filter); >>> - if (cur != org) { >>> + orig = !!(dev->data->dev_conf.rxmode.hw_vlan_filter); >>> + if (cur != orig) { >>> dev->data->dev_conf.rxmode.hw_vlan_filter = (uint8_t)cur; >>> mask |= ETH_VLAN_FILTER_MASK; >>> } >>> >>> cur = !!(offload_mask & ETH_VLAN_EXTEND_OFFLOAD); >>> - org = !!(dev->data->dev_conf.rxmode.hw_vlan_extend); >>> - if (cur != org) { >>> + orig = !!(dev->data->dev_conf.rxmode.hw_vlan_extend); >>> + if (cur != orig) { >>> dev->data->dev_conf.rxmode.hw_vlan_extend = (uint8_t)cur; >>> mask |= ETH_VLAN_EXTEND_MASK; >>> } >>> @@ -2080,7 +2086,13 @@ struct rte_eth_dev * >>> return ret; >>> >>> RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->vlan_offload_set, -ENOTSUP); >>> - (*dev->dev_ops->vlan_offload_set)(dev, mask); >>> + ret = (*dev->dev_ops->vlan_offload_set)(dev, mask); >>> + if (ret) { >>> + /* hit an error restore original values */ >>> + dev->data->dev_conf.rxmode.hw_vlan_strip = orig_strip; >>> + dev->data->dev_conf.rxmode.hw_vlan_filter = orig_filter; >>> + dev->data->dev_conf.rxmode.hw_vlan_extend = orig_extend; >>> + } >>> >> Currently vlan offload is combining three functions: >> strip, filter and extend. >> >> Not all PMDs in DPDK support all of three. >> Should not the error we extended to reflect, which of the VLAN offload >> is not supported? > > There are many examples of APIs that not all PMDs support. > There are also other APIs that manipulate many attributes but do > not communicate which attribute fails on set. > Solving these issues I believe it outside the scope of this change > and it requires a larger community discussion. > > This proposed change at least adheres to the existing paradigm. I agree that solving this issue is outside the scope of this patch. However this patch may leave the drivers in incorrect state. Earlier this API was not returning error or failing. With this change, the driver may want to implement the error handling if 2nd or 3rd attribute failed. Note that you are also assuming and reverting back to the original condition. The change is fine w.r.t dpaa2 driver. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [dpdk-dev] [PATCH v4] ethdev: allow returning error on VLAN offload configuration 2017-09-07 9:37 ` Hemant Agrawal @ 2017-09-07 12:09 ` David Harton (dharton) 0 siblings, 0 replies; 19+ messages in thread From: David Harton (dharton) @ 2017-09-07 12:09 UTC (permalink / raw) To: Hemant Agrawal, thomas, ferruh.yigit, ajit.khaparde, John Daley (johndale), konstantin.ananyev, jingjing.wu, beilei.xing, jing.d.chen, adrien.mazarguil, nelio.laranjeiro, alejandro.lucero, rasesh.mody, harish.patil, skhare, yliu, maxime.coquelin@redhat.com Cc: dev > -----Original Message----- > From: Hemant Agrawal [mailto:hemant.agrawal@nxp.com] > > On 9/1/2017 6:24 PM, David Harton (dharton) wrote: > > > >> -----Original Message----- > >> From: Hemant Agrawal [mailto:hemant.agrawal@nxp.com] > >> > >> On 9/1/2017 8:06 AM, David Harton wrote: > >>> Some devices may not support or fail setting VLAN offload > >>> configuration based on dynamic circurmstances so the > >>> vlan_offload_set_t vector is modified to return an int so the caller > >>> can determine success or not. > >>> > >>> rte_eth_dev_set_vlan_offload is updated to return the value provided > >>> by the vector when called along with restoring the original offload > >>> configs on failure. > >>> > >>> Existing vlan_offload_set_t vectors are modified to return an int. > >>> Majority of cases return 0 but a few that actually can fail now > >>> return their failure codes. > >>> > >>> Finally, a vlan_offload_set_t vector is added to virtio to > >>> facilitate dynamically turning VLAN strip on or off. > >>> > >>> Signed-off-by: David Harton <dharton@cisco.com> > >>> --- > > <snip>.... > > >>> diff --git a/lib/librte_ether/rte_ethdev.c > >> b/lib/librte_ether/rte_ethdev.c > >>> index 0597641..05e52b8 100644 > >>> --- a/lib/librte_ether/rte_ethdev.c > >>> +++ b/lib/librte_ether/rte_ethdev.c > >>> @@ -2048,29 +2048,35 @@ struct rte_eth_dev * > >>> struct rte_eth_dev *dev; > >>> int ret = 0; > >>> int mask = 0; > >>> - int cur, org = 0; > >>> + int cur, orig = 0; > >>> + uint8_t orig_strip, orig_filter, orig_extend; > >>> > >>> RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV); > >>> dev = &rte_eth_devices[port_id]; > >>> > >>> + /* save original values in case of failure */ > >>> + orig_strip = dev->data->dev_conf.rxmode.hw_vlan_strip; > >>> + orig_filter = dev->data->dev_conf.rxmode.hw_vlan_filter; > >>> + orig_extend = dev->data->dev_conf.rxmode.hw_vlan_extend; > >>> + > >>> /*check which option changed by application*/ > >>> cur = !!(offload_mask & ETH_VLAN_STRIP_OFFLOAD); > >>> - org = !!(dev->data->dev_conf.rxmode.hw_vlan_strip); > >>> - if (cur != org) { > >>> + orig = !!(dev->data->dev_conf.rxmode.hw_vlan_strip); > >>> + if (cur != orig) { > >>> dev->data->dev_conf.rxmode.hw_vlan_strip = (uint8_t)cur; > >>> mask |= ETH_VLAN_STRIP_MASK; > >>> } > >>> > >>> cur = !!(offload_mask & ETH_VLAN_FILTER_OFFLOAD); > >>> - org = !!(dev->data->dev_conf.rxmode.hw_vlan_filter); > >>> - if (cur != org) { > >>> + orig = !!(dev->data->dev_conf.rxmode.hw_vlan_filter); > >>> + if (cur != orig) { > >>> dev->data->dev_conf.rxmode.hw_vlan_filter = (uint8_t)cur; > >>> mask |= ETH_VLAN_FILTER_MASK; > >>> } > >>> > >>> cur = !!(offload_mask & ETH_VLAN_EXTEND_OFFLOAD); > >>> - org = !!(dev->data->dev_conf.rxmode.hw_vlan_extend); > >>> - if (cur != org) { > >>> + orig = !!(dev->data->dev_conf.rxmode.hw_vlan_extend); > >>> + if (cur != orig) { > >>> dev->data->dev_conf.rxmode.hw_vlan_extend = (uint8_t)cur; > >>> mask |= ETH_VLAN_EXTEND_MASK; > >>> } > >>> @@ -2080,7 +2086,13 @@ struct rte_eth_dev * > >>> return ret; > >>> > >>> RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->vlan_offload_set, -ENOTSUP); > >>> - (*dev->dev_ops->vlan_offload_set)(dev, mask); > >>> + ret = (*dev->dev_ops->vlan_offload_set)(dev, mask); > >>> + if (ret) { > >>> + /* hit an error restore original values */ > >>> + dev->data->dev_conf.rxmode.hw_vlan_strip = orig_strip; > >>> + dev->data->dev_conf.rxmode.hw_vlan_filter = orig_filter; > >>> + dev->data->dev_conf.rxmode.hw_vlan_extend = orig_extend; > >>> + } > >>> > >> Currently vlan offload is combining three functions: > >> strip, filter and extend. > >> > >> Not all PMDs in DPDK support all of three. > >> Should not the error we extended to reflect, which of the VLAN > >> offload is not supported? > > > > There are many examples of APIs that not all PMDs support. > > There are also other APIs that manipulate many attributes but do not > > communicate which attribute fails on set. > > Solving these issues I believe it outside the scope of this change and > > it requires a larger community discussion. > > > > This proposed change at least adheres to the existing paradigm. > > I agree that solving this issue is outside the scope of this patch. > > However this patch may leave the drivers in incorrect state. I agree. When failures happen in other APIs as well, whether reported to the caller or not, how the failure is handled is inconsistent. Also, drivers could be in the incorrect state after this API call with the current implementation. > > Earlier this API was not returning error or failing. With this change, the > driver may want to implement the error handling if 2nd or 3rd attribute > failed. Note that you are also assuming and reverting back to the original > condition. The real thrust of this change is to report the failure so the caller can attempt to take some corrective action. Is there a policy one way or the other on whether DPDK restores original state on failures or just leaves the system in a failed state and reports the condition? I've always come from the ilk that if an API call fails then the state of the system should, as much as possible, remain the same as prior to the API call which is why I restored the flags on failure. With that, I can make ethdev call the device again with the "restored" values and still return the first failure. Or I could leave ethdev alone and leave the "bad state" as the previous implementation did? > > The change is fine w.r.t dpaa2 driver. Thanks. And thanks for the good discussion. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [dpdk-dev] [PATCH v4] ethdev: allow returning error on VLAN offload configuration 2017-09-01 2:36 ` [dpdk-dev] [PATCH v4] ethdev: allow returning error on VLAN offload configuration David Harton 2017-09-01 7:41 ` Hemant Agrawal @ 2017-10-10 5:34 ` Ferruh Yigit 2017-10-10 12:20 ` David Harton (dharton) 2017-10-25 3:01 ` [dpdk-dev] [PATCH v5] ethdev: allow returning error on VLAN offload ops Ferruh Yigit 2 siblings, 1 reply; 19+ messages in thread From: Ferruh Yigit @ 2017-10-10 5:34 UTC (permalink / raw) To: David Harton, thomas, ajit.khaparde, johndale, konstantin.ananyev, jingjing.wu, beilei.xing, jing.d.chen, adrien.mazarguil, nelio.laranjeiro, alejandro.lucero, hemant.agrawal, rasesh.mody, harish.patil, skhare, yliu, maxime.coquelin, allain.legacy Cc: dev On 9/1/2017 3:36 AM, David Harton wrote: > Some devices may not support or fail setting VLAN offload > configuration based on dynamic circurmstances so the > vlan_offload_set_t vector is modified to return an int so > the caller can determine success or not. > > rte_eth_dev_set_vlan_offload is updated to return the > value provided by the vector when called along with restoring > the original offload configs on failure. > > Existing vlan_offload_set_t vectors are modified to return > an int. Majority of cases return 0 but a few that actually > can fail now return their failure codes. > > Finally, a vlan_offload_set_t vector is added to virtio > to facilitate dynamically turning VLAN strip on or off. > > Signed-off-by: David Harton <dharton@cisco.com> Hi David, This patch conflicts against latest version, would you mind rebasing it on top of latest next-net? Thanks, ferruh ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [dpdk-dev] [PATCH v4] ethdev: allow returning error on VLAN offload configuration 2017-10-10 5:34 ` Ferruh Yigit @ 2017-10-10 12:20 ` David Harton (dharton) 0 siblings, 0 replies; 19+ messages in thread From: David Harton (dharton) @ 2017-10-10 12:20 UTC (permalink / raw) To: Ferruh Yigit, thomas, ajit.khaparde, John Daley (johndale), konstantin.ananyev, jingjing.wu, beilei.xing, jing.d.chen, adrien.mazarguil, nelio.laranjeiro, alejandro.lucero, hemant.agrawal, rasesh.mody, harish.patil, skhare, yliu, maxime.coquelin, allain.legacy Cc: dev > -----Original Message----- > From: Ferruh Yigit [mailto:ferruh.yigit@intel.com] > > On 9/1/2017 3:36 AM, David Harton wrote: > > Some devices may not support or fail setting VLAN offload > > configuration based on dynamic circurmstances so the > > vlan_offload_set_t vector is modified to return an int so the caller > > can determine success or not. > > > > rte_eth_dev_set_vlan_offload is updated to return the value provided > > by the vector when called along with restoring the original offload > > configs on failure. > > > > Existing vlan_offload_set_t vectors are modified to return an int. > > Majority of cases return 0 but a few that actually can fail now return > > their failure codes. > > > > Finally, a vlan_offload_set_t vector is added to virtio to facilitate > > dynamically turning VLAN strip on or off. > > > > Signed-off-by: David Harton <dharton@cisco.com> > > Hi David, > > This patch conflicts against latest version, would you mind rebasing it on > top of latest next-net? Sure Ferruh, no problem. Thanks, Dave > > Thanks, > ferruh ^ permalink raw reply [flat|nested] 19+ messages in thread
* [dpdk-dev] [PATCH v5] ethdev: allow returning error on VLAN offload ops 2017-09-01 2:36 ` [dpdk-dev] [PATCH v4] ethdev: allow returning error on VLAN offload configuration David Harton 2017-09-01 7:41 ` Hemant Agrawal 2017-10-10 5:34 ` Ferruh Yigit @ 2017-10-25 3:01 ` Ferruh Yigit 2017-10-25 22:13 ` Ferruh Yigit 2 siblings, 1 reply; 19+ messages in thread From: Ferruh Yigit @ 2017-10-25 3:01 UTC (permalink / raw) To: John McNamara, Allain Legacy, Matt Peters, Stephen Hurd, Ajit Khaparde, Hemant Agrawal, Shreyansh Jain, Wenzhuo Lu, John Daley, Nelson Escobar, Jing Chen, Jingjing Wu, Beilei Xing, Konstantin Ananyev, Adrien Mazarguil, Nelio Laranjeiro, Yongseok Koh, Alejandro Lucero, Rasesh Mody, Harish Patil, Shahed Shaikh, Yuanhan Liu, Maxime Coquelin, Shrikrishna Khare, Thomas Monjalon Cc: dev, Ferruh Yigit, David Harton From: David Harton <dharton@cisco.com> Some devices may not support or fail setting VLAN offload configuration based on dynamic circumstances so the vlan_offload_set_t vector is modified to return an int so the caller can determine success or not. rte_eth_dev_set_vlan_offload is updated to return the value provided by the vector when called along with restoring the original offload configs on failure. Existing vlan_offload_set_t vectors are modified to return an int. Majority of cases return 0 but a few that actually can fail now return their failure codes. Finally, a vlan_offload_set_t vector is added to virtio to facilitate dynamically turning VLAN strip on or off. Signed-off-by: David Harton <dharton@cisco.com> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com> --- v5 * rebase on latest next-net * add ETH_VLAN_FILTER_MASK mask check to virtio v4 * Modified commit message heading * Moved rel_note comments from ABI to API section * Renamed locals of rte_eth_dev_set_vlan_offload from 'org*' to 'orig*' v3 * Fixed a format error. * Apologies...need to figure out why checkpatches.pl keeps saying valid patch when I've got soft tabs. v2 * Fixed a missed format error. * Removed vlan offload vector call casts and replaced with checks for return values. v1 * This is an ABI breakage that has been previously negotiated with Thomas and the proposed rel note change is included as well. --- doc/guides/rel_notes/release_17_11.rst | 7 +++++++ drivers/net/avp/avp_ethdev.c | 13 ++++++++++--- drivers/net/bnxt/bnxt_ethdev.c | 10 +++++++--- drivers/net/dpaa2/dpaa2_ethdev.c | 15 ++++++++++++--- drivers/net/e1000/em_ethdev.c | 13 ++++++++++--- drivers/net/e1000/igb_ethdev.c | 13 ++++++++++--- drivers/net/enic/enic_ethdev.c | 9 ++++++--- drivers/net/fm10k/fm10k_ethdev.c | 4 +++- drivers/net/i40e/i40e_ethdev.c | 12 +++++++++--- drivers/net/i40e/i40e_ethdev_vf.c | 10 +++++----- drivers/net/ixgbe/ixgbe_ethdev.c | 27 ++++++++++++++++++++------- drivers/net/mlx5/mlx5.h | 2 +- drivers/net/mlx5/mlx5_vlan.c | 6 ++++-- drivers/net/nfp/nfp_net.c | 12 +++++++----- drivers/net/qede/qede_ethdev.c | 9 +++++++-- drivers/net/virtio/virtio_ethdev.c | 25 +++++++++++++++++++++++++ drivers/net/vmxnet3/vmxnet3_ethdev.c | 12 ++++++++---- lib/librte_ether/rte_ethdev.c | 12 +++++++++++- lib/librte_ether/rte_ethdev.h | 2 +- 19 files changed, 163 insertions(+), 50 deletions(-) diff --git a/doc/guides/rel_notes/release_17_11.rst b/doc/guides/rel_notes/release_17_11.rst index e505a4dcd..65ab4b767 100644 --- a/doc/guides/rel_notes/release_17_11.rst +++ b/doc/guides/rel_notes/release_17_11.rst @@ -322,6 +322,13 @@ API Changes ``rte_log_get_global_level()``, ``rte_log_set_level()`` and ``rte_log_get_level()``. +* **Modified the vlan_offload_set_t function prototype in the ethdev library.** + + Changed the function prototype of ``vlan_offload_set_t``. The return value + has been changed from ``void`` to ``int`` so the caller to knows whether + the backing device supports the operation or if the operation was + successfully performed. + ABI Changes ----------- diff --git a/drivers/net/avp/avp_ethdev.c b/drivers/net/avp/avp_ethdev.c index b97a90cea..7853fbf15 100644 --- a/drivers/net/avp/avp_ethdev.c +++ b/drivers/net/avp/avp_ethdev.c @@ -70,7 +70,7 @@ static void avp_dev_stop(struct rte_eth_dev *dev); static void avp_dev_close(struct rte_eth_dev *dev); static void avp_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info); -static void avp_vlan_offload_set(struct rte_eth_dev *dev, int mask); +static int avp_vlan_offload_set(struct rte_eth_dev *dev, int mask); static int avp_dev_link_update(struct rte_eth_dev *dev, int wait_to_complete); static void avp_dev_promiscuous_enable(struct rte_eth_dev *dev); static void avp_dev_promiscuous_disable(struct rte_eth_dev *dev); @@ -2031,7 +2031,12 @@ avp_dev_configure(struct rte_eth_dev *eth_dev) mask = (ETH_VLAN_STRIP_MASK | ETH_VLAN_FILTER_MASK | ETH_VLAN_EXTEND_MASK); - avp_vlan_offload_set(eth_dev, mask); + ret = avp_vlan_offload_set(eth_dev, mask); + if (ret < 0) { + PMD_DRV_LOG(ERR, "VLAN offload set failed by host, ret=%d\n", + ret); + goto unlock; + } /* update device config */ memset(&config, 0, sizeof(config)); @@ -2214,7 +2219,7 @@ avp_dev_info_get(struct rte_eth_dev *eth_dev, } } -static void +static int avp_vlan_offload_set(struct rte_eth_dev *eth_dev, int mask) { struct avp_dev *avp = AVP_DEV_PRIVATE_TO_HW(eth_dev->data->dev_private); @@ -2239,6 +2244,8 @@ avp_vlan_offload_set(struct rte_eth_dev *eth_dev, int mask) if (eth_dev->data->dev_conf.rxmode.hw_vlan_extend) PMD_DRV_LOG(ERR, "VLAN extend offload not supported\n"); } + + return 0; } static int diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c index c2d54efd8..ad50045ac 100644 --- a/drivers/net/bnxt/bnxt_ethdev.c +++ b/drivers/net/bnxt/bnxt_ethdev.c @@ -145,7 +145,7 @@ static const struct rte_pci_id bnxt_pci_id_map[] = { ETH_RSS_NONFRAG_IPV6_TCP | \ ETH_RSS_NONFRAG_IPV6_UDP) -static void bnxt_vlan_offload_set_op(struct rte_eth_dev *dev, int mask); +static int bnxt_vlan_offload_set_op(struct rte_eth_dev *dev, int mask); /***********************/ @@ -591,7 +591,9 @@ static int bnxt_dev_start_op(struct rte_eth_dev *eth_dev) vlan_mask |= ETH_VLAN_FILTER_MASK; if (eth_dev->data->dev_conf.rxmode.hw_vlan_strip) vlan_mask |= ETH_VLAN_STRIP_MASK; - bnxt_vlan_offload_set_op(eth_dev, vlan_mask); + rc = bnxt_vlan_offload_set_op(eth_dev, vlan_mask); + if (rc) + goto error; return 0; @@ -1350,7 +1352,7 @@ static int bnxt_vlan_filter_set_op(struct rte_eth_dev *eth_dev, return bnxt_del_vlan_filter(bp, vlan_id); } -static void +static int bnxt_vlan_offload_set_op(struct rte_eth_dev *dev, int mask) { struct bnxt *bp = (struct bnxt *)dev->data->dev_private; @@ -1382,6 +1384,8 @@ bnxt_vlan_offload_set_op(struct rte_eth_dev *dev, int mask) if (mask & ETH_VLAN_EXTEND_MASK) RTE_LOG(ERR, PMD, "Extend VLAN Not supported\n"); + + return 0; } static void diff --git a/drivers/net/dpaa2/dpaa2_ethdev.c b/drivers/net/dpaa2/dpaa2_ethdev.c index 1ac607cb0..881a209d8 100644 --- a/drivers/net/dpaa2/dpaa2_ethdev.c +++ b/drivers/net/dpaa2/dpaa2_ethdev.c @@ -162,7 +162,7 @@ dpaa2_vlan_filter_set(struct rte_eth_dev *dev, uint16_t vlan_id, int on) return ret; } -static void +static int dpaa2_vlan_offload_set(struct rte_eth_dev *dev, int mask) { struct dpaa2_dev_priv *priv = dev->data->dev_private; @@ -188,6 +188,8 @@ dpaa2_vlan_offload_set(struct rte_eth_dev *dev, int mask) RTE_LOG(INFO, PMD, "VLAN extend offload not supported\n"); } + + return 0; } static int @@ -754,8 +756,15 @@ dpaa2_dev_start(struct rte_eth_dev *dev) return ret; } /* VLAN Offload Settings */ - if (priv->max_vlan_filters) - dpaa2_vlan_offload_set(dev, ETH_VLAN_FILTER_MASK); + if (priv->max_vlan_filters) { + ret = dpaa2_vlan_offload_set(dev, ETH_VLAN_FILTER_MASK); + if (ret) { + PMD_INIT_LOG(ERR, "Error to dpaa2_vlan_offload_set:" + "code = %d\n", ret); + return ret; + } + } + /* if the interrupts were configured on this devices*/ if (intr_handle && (intr_handle->fd) && diff --git a/drivers/net/e1000/em_ethdev.c b/drivers/net/e1000/em_ethdev.c index 6ebfa6d4c..770ab8141 100644 --- a/drivers/net/e1000/em_ethdev.c +++ b/drivers/net/e1000/em_ethdev.c @@ -99,7 +99,7 @@ static int eth_em_mtu_set(struct rte_eth_dev *dev, uint16_t mtu); static int eth_em_vlan_filter_set(struct rte_eth_dev *dev, uint16_t vlan_id, int on); -static void eth_em_vlan_offload_set(struct rte_eth_dev *dev, int mask); +static int eth_em_vlan_offload_set(struct rte_eth_dev *dev, int mask); static void em_vlan_hw_filter_enable(struct rte_eth_dev *dev); static void em_vlan_hw_filter_disable(struct rte_eth_dev *dev); static void em_vlan_hw_strip_enable(struct rte_eth_dev *dev); @@ -669,7 +669,12 @@ eth_em_start(struct rte_eth_dev *dev) mask = ETH_VLAN_STRIP_MASK | ETH_VLAN_FILTER_MASK | \ ETH_VLAN_EXTEND_MASK; - eth_em_vlan_offload_set(dev, mask); + ret = eth_em_vlan_offload_set(dev, mask); + if (ret) { + PMD_INIT_LOG(ERR, "Unable to update vlan offload"); + em_dev_clear_queues(dev); + return ret; + } /* Set Interrupt Throttling Rate to maximum allowed value. */ E1000_WRITE_REG(hw, E1000_ITR, UINT16_MAX); @@ -1449,7 +1454,7 @@ em_vlan_hw_strip_enable(struct rte_eth_dev *dev) E1000_WRITE_REG(hw, E1000_CTRL, reg); } -static void +static int eth_em_vlan_offload_set(struct rte_eth_dev *dev, int mask) { if(mask & ETH_VLAN_STRIP_MASK){ @@ -1465,6 +1470,8 @@ eth_em_vlan_offload_set(struct rte_eth_dev *dev, int mask) else em_vlan_hw_filter_disable(dev); } + + return 0; } /* diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c index 003bdf0f6..53f160030 100644 --- a/drivers/net/e1000/igb_ethdev.c +++ b/drivers/net/e1000/igb_ethdev.c @@ -157,7 +157,7 @@ static int eth_igb_vlan_filter_set(struct rte_eth_dev *dev, static int eth_igb_vlan_tpid_set(struct rte_eth_dev *dev, enum rte_vlan_type vlan_type, uint16_t tpid_id); -static void eth_igb_vlan_offload_set(struct rte_eth_dev *dev, int mask); +static int eth_igb_vlan_offload_set(struct rte_eth_dev *dev, int mask); static void igb_vlan_hw_filter_enable(struct rte_eth_dev *dev); static void igb_vlan_hw_filter_disable(struct rte_eth_dev *dev); @@ -1403,7 +1403,12 @@ eth_igb_start(struct rte_eth_dev *dev) */ mask = ETH_VLAN_STRIP_MASK | ETH_VLAN_FILTER_MASK | \ ETH_VLAN_EXTEND_MASK; - eth_igb_vlan_offload_set(dev, mask); + ret = eth_igb_vlan_offload_set(dev, mask); + if (ret) { + PMD_INIT_LOG(ERR, "Unable to set vlan offload"); + igb_dev_clear_queues(dev); + return ret; + } if (dev->data->dev_conf.rxmode.mq_mode == ETH_MQ_RX_VMDQ_ONLY) { /* Enable VLAN filter since VMDq always use VLAN filter */ @@ -2720,7 +2725,7 @@ igb_vlan_hw_extend_enable(struct rte_eth_dev *dev) 2 * VLAN_TAG_SIZE); } -static void +static int eth_igb_vlan_offload_set(struct rte_eth_dev *dev, int mask) { if(mask & ETH_VLAN_STRIP_MASK){ @@ -2743,6 +2748,8 @@ eth_igb_vlan_offload_set(struct rte_eth_dev *dev, int mask) else igb_vlan_hw_extend_disable(dev); } + + return 0; } diff --git a/drivers/net/enic/enic_ethdev.c b/drivers/net/enic/enic_ethdev.c index 5386b2ae3..c02f9b7b3 100644 --- a/drivers/net/enic/enic_ethdev.c +++ b/drivers/net/enic/enic_ethdev.c @@ -362,7 +362,7 @@ static int enicpmd_vlan_filter_set(struct rte_eth_dev *eth_dev, return err; } -static void enicpmd_vlan_offload_set(struct rte_eth_dev *eth_dev, int mask) +static int enicpmd_vlan_offload_set(struct rte_eth_dev *eth_dev, int mask) { struct enic *enic = pmd_priv(eth_dev); @@ -386,6 +386,8 @@ static void enicpmd_vlan_offload_set(struct rte_eth_dev *eth_dev, int mask) dev_warning(enic, "Configuration of extended VLAN is not supported\n"); } + + return 0; } static int enicpmd_dev_configure(struct rte_eth_dev *eth_dev) @@ -410,9 +412,10 @@ static int enicpmd_dev_configure(struct rte_eth_dev *eth_dev) eth_dev->data->dev_conf.rxmode.split_hdr_size); } - enicpmd_vlan_offload_set(eth_dev, ETH_VLAN_STRIP_MASK); enic->hw_ip_checksum = eth_dev->data->dev_conf.rxmode.hw_ip_checksum; - return 0; + ret = enicpmd_vlan_offload_set(eth_dev, ETH_VLAN_STRIP_MASK); + + return ret; } /* Start the device. diff --git a/drivers/net/fm10k/fm10k_ethdev.c b/drivers/net/fm10k/fm10k_ethdev.c index 7baa9279d..46a312fad 100644 --- a/drivers/net/fm10k/fm10k_ethdev.c +++ b/drivers/net/fm10k/fm10k_ethdev.c @@ -1594,7 +1594,7 @@ fm10k_vlan_filter_set(struct rte_eth_dev *dev, uint16_t vlan_id, int on) return 0; } -static void +static int fm10k_vlan_offload_set(struct rte_eth_dev *dev, int mask) { if (mask & ETH_VLAN_STRIP_MASK) { @@ -1613,6 +1613,8 @@ fm10k_vlan_offload_set(struct rte_eth_dev *dev, int mask) if (!dev->data->dev_conf.rxmode.hw_vlan_filter) PMD_INIT_LOG(ERR, "VLAN filter is always on in fm10k"); } + + return 0; } /* Add/Remove a MAC address, and update filters to main VSI */ diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c index 48db2dde3..86c43881b 100644 --- a/drivers/net/i40e/i40e_ethdev.c +++ b/drivers/net/i40e/i40e_ethdev.c @@ -276,7 +276,7 @@ static int i40e_vlan_filter_set(struct rte_eth_dev *dev, static int i40e_vlan_tpid_set(struct rte_eth_dev *dev, enum rte_vlan_type vlan_type, uint16_t tpid); -static void i40e_vlan_offload_set(struct rte_eth_dev *dev, int mask); +static int i40e_vlan_offload_set(struct rte_eth_dev *dev, int mask); static void i40e_vlan_strip_queue_set(struct rte_eth_dev *dev, uint16_t queue, int on); @@ -3221,7 +3221,7 @@ i40e_vlan_tpid_set(struct rte_eth_dev *dev, return ret; } -static void +static int i40e_vlan_offload_set(struct rte_eth_dev *dev, int mask) { struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private); @@ -3254,6 +3254,8 @@ i40e_vlan_offload_set(struct rte_eth_dev *dev, int mask) else i40e_vsi_config_double_vlan(vsi, FALSE); } + + return 0; } static void @@ -5314,7 +5316,11 @@ i40e_dev_init_vlan(struct rte_eth_dev *dev) /* Apply vlan offload setting */ mask = ETH_VLAN_STRIP_MASK | ETH_VLAN_FILTER_MASK; - i40e_vlan_offload_set(dev, mask); + ret = i40e_vlan_offload_set(dev, mask); + if (ret) { + PMD_DRV_LOG(INFO, "Failed to update vlan offload"); + return ret; + } /* Apply double-vlan setting, not implemented yet */ diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c index 9f1487509..ca1d9f930 100644 --- a/drivers/net/i40e/i40e_ethdev_vf.c +++ b/drivers/net/i40e/i40e_ethdev_vf.c @@ -118,7 +118,7 @@ static int i40evf_dev_xstats_get_names(struct rte_eth_dev *dev, static void i40evf_dev_xstats_reset(struct rte_eth_dev *dev); static int i40evf_vlan_filter_set(struct rte_eth_dev *dev, uint16_t vlan_id, int on); -static void i40evf_vlan_offload_set(struct rte_eth_dev *dev, int mask); +static int i40evf_vlan_offload_set(struct rte_eth_dev *dev, int mask); static void i40evf_dev_close(struct rte_eth_dev *dev); static int i40evf_dev_reset(struct rte_eth_dev *dev); static void i40evf_dev_promiscuous_enable(struct rte_eth_dev *dev); @@ -1588,12 +1588,10 @@ static int i40evf_init_vlan(struct rte_eth_dev *dev) { /* Apply vlan offload setting */ - i40evf_vlan_offload_set(dev, ETH_VLAN_STRIP_MASK); - - return I40E_SUCCESS; + return i40evf_vlan_offload_set(dev, ETH_VLAN_STRIP_MASK); } -static void +static int i40evf_vlan_offload_set(struct rte_eth_dev *dev, int mask) { struct rte_eth_conf *dev_conf = &dev->data->dev_conf; @@ -1606,6 +1604,8 @@ i40evf_vlan_offload_set(struct rte_eth_dev *dev, int mask) else i40evf_disable_vlan_strip(dev); } + + return 0; } static int diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c index 2ff904119..371998e42 100644 --- a/drivers/net/ixgbe/ixgbe_ethdev.c +++ b/drivers/net/ixgbe/ixgbe_ethdev.c @@ -219,7 +219,7 @@ static void ixgbe_vlan_hw_strip_bitmap_set(struct rte_eth_dev *dev, uint16_t queue, bool on); static void ixgbe_vlan_strip_queue_set(struct rte_eth_dev *dev, uint16_t queue, int on); -static void ixgbe_vlan_offload_set(struct rte_eth_dev *dev, int mask); +static int ixgbe_vlan_offload_set(struct rte_eth_dev *dev, int mask); static void ixgbe_vlan_hw_strip_enable(struct rte_eth_dev *dev, uint16_t queue); static void ixgbe_vlan_hw_strip_disable(struct rte_eth_dev *dev, uint16_t queue); static void ixgbe_vlan_hw_extend_enable(struct rte_eth_dev *dev); @@ -276,7 +276,7 @@ static int ixgbevf_vlan_filter_set(struct rte_eth_dev *dev, uint16_t vlan_id, int on); static void ixgbevf_vlan_strip_queue_set(struct rte_eth_dev *dev, uint16_t queue, int on); -static void ixgbevf_vlan_offload_set(struct rte_eth_dev *dev, int mask); +static int ixgbevf_vlan_offload_set(struct rte_eth_dev *dev, int mask); static void ixgbevf_set_vfta_all(struct rte_eth_dev *dev, bool on); static int ixgbevf_dev_rx_queue_intr_enable(struct rte_eth_dev *dev, uint16_t queue_id); @@ -2126,7 +2126,7 @@ ixgbe_vlan_hw_extend_enable(struct rte_eth_dev *dev) */ } -static void +static int ixgbe_vlan_offload_set(struct rte_eth_dev *dev, int mask) { if (mask & ETH_VLAN_STRIP_MASK) { @@ -2149,6 +2149,8 @@ ixgbe_vlan_offload_set(struct rte_eth_dev *dev, int mask) else ixgbe_vlan_hw_extend_disable(dev); } + + return 0; } static void @@ -2570,9 +2572,13 @@ ixgbe_dev_start(struct rte_eth_dev *dev) goto error; } - mask = ETH_VLAN_STRIP_MASK | ETH_VLAN_FILTER_MASK | + mask = ETH_VLAN_STRIP_MASK | ETH_VLAN_FILTER_MASK | ETH_VLAN_EXTEND_MASK; - ixgbe_vlan_offload_set(dev, mask); + err = ixgbe_vlan_offload_set(dev, mask); + if (err) { + PMD_INIT_LOG(ERR, "Unable to set VLAN offload"); + goto error; + } if (dev->data->dev_conf.rxmode.mq_mode == ETH_MQ_RX_VMDQ_ONLY) { /* Enable vlan filtering for VMDq */ @@ -5024,7 +5030,12 @@ ixgbevf_dev_start(struct rte_eth_dev *dev) /* Set HW strip */ mask = ETH_VLAN_STRIP_MASK | ETH_VLAN_FILTER_MASK | ETH_VLAN_EXTEND_MASK; - ixgbevf_vlan_offload_set(dev, mask); + err = ixgbevf_vlan_offload_set(dev, mask); + if (err) { + PMD_INIT_LOG(ERR, "Unable to set VLAN offload (%d)", err); + ixgbe_dev_clear_queues(dev); + return err; + } ixgbevf_dev_rxtx_start(dev); @@ -5213,7 +5224,7 @@ ixgbevf_vlan_strip_queue_set(struct rte_eth_dev *dev, uint16_t queue, int on) ixgbe_vlan_hw_strip_bitmap_set(dev, queue, on); } -static void +static int ixgbevf_vlan_offload_set(struct rte_eth_dev *dev, int mask) { struct ixgbe_hw *hw = @@ -5228,6 +5239,8 @@ ixgbevf_vlan_offload_set(struct rte_eth_dev *dev, int mask) for (i = 0; i < hw->mac.max_rx_queues; i++) ixgbevf_vlan_strip_queue_set(dev, i, on); } + + return 0; } int diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h index 3f452b2f0..e6a69b823 100644 --- a/drivers/net/mlx5/mlx5.h +++ b/drivers/net/mlx5/mlx5.h @@ -248,7 +248,7 @@ int mlx5_xstats_get_names(struct rte_eth_dev *, /* mlx5_vlan.c */ int mlx5_vlan_filter_set(struct rte_eth_dev *, uint16_t, int); -void mlx5_vlan_offload_set(struct rte_eth_dev *, int); +int mlx5_vlan_offload_set(struct rte_eth_dev *, int); void mlx5_vlan_strip_queue_set(struct rte_eth_dev *, uint16_t, int); /* mlx5_trigger.c */ diff --git a/drivers/net/mlx5/mlx5_vlan.c b/drivers/net/mlx5/mlx5_vlan.c index ed91d9b28..89874aabd 100644 --- a/drivers/net/mlx5/mlx5_vlan.c +++ b/drivers/net/mlx5/mlx5_vlan.c @@ -178,7 +178,7 @@ mlx5_vlan_strip_queue_set(struct rte_eth_dev *dev, uint16_t queue, int on) * @param mask * VLAN offload bit mask. */ -void +int mlx5_vlan_offload_set(struct rte_eth_dev *dev, int mask) { struct priv *priv = dev->data->dev_private; @@ -189,7 +189,7 @@ mlx5_vlan_offload_set(struct rte_eth_dev *dev, int mask) if (!priv->hw_vlan_strip) { ERROR("VLAN stripping is not supported"); - return; + return 0; } /* Run on every RX queue and set/reset VLAN stripping. */ @@ -198,4 +198,6 @@ mlx5_vlan_offload_set(struct rte_eth_dev *dev, int mask) priv_vlan_strip_queue_set(priv, i, hw_vlan_strip); priv_unlock(priv); } + + return 0; } diff --git a/drivers/net/nfp/nfp_net.c b/drivers/net/nfp/nfp_net.c index 0917b9c49..71d4a8629 100644 --- a/drivers/net/nfp/nfp_net.c +++ b/drivers/net/nfp/nfp_net.c @@ -2306,11 +2306,12 @@ nfp_net_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts, uint16_t nb_pkts) return i; } -static void +static int nfp_net_vlan_offload_set(struct rte_eth_dev *dev, int mask) { uint32_t new_ctrl, update; struct nfp_net_hw *hw; + int ret; hw = NFP_NET_DEV_PRIVATE_TO_HW(dev->data->dev_private); new_ctrl = 0; @@ -2331,14 +2332,15 @@ nfp_net_vlan_offload_set(struct rte_eth_dev *dev, int mask) new_ctrl = hw->ctrl & ~NFP_NET_CFG_CTRL_RXVLAN; if (new_ctrl == 0) - return; + return 0; update = NFP_NET_CFG_UPDATE_GEN; - if (nfp_net_reconfig(hw, new_ctrl, update) < 0) - return; + ret = nfp_net_reconfig(hw, new_ctrl, update); + if (!ret) + hw->ctrl = new_ctrl; - hw->ctrl = new_ctrl; + return ret; } /* Update Redirection Table(RETA) of Receive Side Scaling of Ethernet device */ diff --git a/drivers/net/qede/qede_ethdev.c b/drivers/net/qede/qede_ethdev.c index 8b3f0ff7f..661d9381e 100644 --- a/drivers/net/qede/qede_ethdev.c +++ b/drivers/net/qede/qede_ethdev.c @@ -1005,7 +1005,7 @@ static int qede_vlan_filter_set(struct rte_eth_dev *eth_dev, return rc; } -static void qede_vlan_offload_set(struct rte_eth_dev *eth_dev, int mask) +static int qede_vlan_offload_set(struct rte_eth_dev *eth_dev, int mask) { struct qede_dev *qdev = QEDE_INIT_QDEV(eth_dev); struct ecore_dev *edev = QEDE_INIT_EDEV(qdev); @@ -1043,6 +1043,8 @@ static void qede_vlan_offload_set(struct rte_eth_dev *eth_dev, int mask) DP_INFO(edev, "vlan offload mask %d vlan-strip %d vlan-filter %d\n", mask, rxmode->hw_vlan_strip, rxmode->hw_vlan_filter); + + return 0; } static void qede_prandom_bytes(uint32_t *buff) @@ -1192,6 +1194,7 @@ static int qede_dev_configure(struct rte_eth_dev *eth_dev) struct qede_dev *qdev = QEDE_INIT_QDEV(eth_dev); struct ecore_dev *edev = QEDE_INIT_EDEV(qdev); struct rte_eth_rxmode *rxmode = ð_dev->data->dev_conf.rxmode; + int ret; PMD_INIT_FUNC_TRACE(edev); @@ -1262,9 +1265,11 @@ static int qede_dev_configure(struct rte_eth_dev *eth_dev) qdev->new_mtu = qdev->mtu; /* Enable VLAN offloads by default */ - qede_vlan_offload_set(eth_dev, ETH_VLAN_STRIP_MASK | + ret = qede_vlan_offload_set(eth_dev, ETH_VLAN_STRIP_MASK | ETH_VLAN_FILTER_MASK | ETH_VLAN_EXTEND_MASK); + if (ret) + return ret; DP_INFO(edev, "Device configured with RSS=%d TSS=%d\n", QEDE_RSS_COUNT(qdev), QEDE_TSS_COUNT(qdev)); diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c index bfbd73770..2e4a49ea9 100644 --- a/drivers/net/virtio/virtio_ethdev.c +++ b/drivers/net/virtio/virtio_ethdev.c @@ -73,6 +73,7 @@ static void virtio_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info); static int virtio_dev_link_update(struct rte_eth_dev *dev, int wait_to_complete); +static int virtio_dev_vlan_offload_set(struct rte_eth_dev *dev, int mask); static void virtio_set_hwaddr(struct virtio_hw *hw); static void virtio_get_hwaddr(struct virtio_hw *hw); @@ -779,6 +780,7 @@ static const struct eth_dev_ops virtio_eth_dev_ops = { .stats_reset = virtio_dev_stats_reset, .xstats_reset = virtio_dev_stats_reset, .link_update = virtio_dev_link_update, + .vlan_offload_set = virtio_dev_vlan_offload_set, .rx_queue_setup = virtio_dev_rx_queue_setup, .rx_queue_intr_enable = virtio_dev_rx_queue_intr_enable, .rx_queue_intr_disable = virtio_dev_rx_queue_intr_disable, @@ -1953,6 +1955,29 @@ virtio_dev_link_update(struct rte_eth_dev *dev, __rte_unused int wait_to_complet return (old.link_status == link.link_status) ? -1 : 0; } +static int +virtio_dev_vlan_offload_set(struct rte_eth_dev *dev, int mask) +{ + const struct rte_eth_rxmode *rxmode = &dev->data->dev_conf.rxmode; + struct virtio_hw *hw = dev->data->dev_private; + + if (mask & ETH_VLAN_FILTER_MASK) { + if (rxmode->hw_vlan_filter && + !vtpci_with_feature(hw, VIRTIO_NET_F_CTRL_VLAN)) { + + PMD_DRV_LOG(NOTICE, + "vlan filtering not available on this host"); + + return -ENOTSUP; + } + } + + if (mask & ETH_VLAN_STRIP_MASK) + hw->vlan_strip = rxmode->hw_vlan_strip; + + return 0; +} + static void virtio_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info) { diff --git a/drivers/net/vmxnet3/vmxnet3_ethdev.c b/drivers/net/vmxnet3/vmxnet3_ethdev.c index 58bc4f2f2..cde112301 100644 --- a/drivers/net/vmxnet3/vmxnet3_ethdev.c +++ b/drivers/net/vmxnet3/vmxnet3_ethdev.c @@ -100,7 +100,7 @@ static const uint32_t * 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 void vmxnet3_dev_vlan_offload_set(struct rte_eth_dev *dev, int mask); +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, struct ether_addr *mac_addr); static void vmxnet3_interrupt_handler(void *param); @@ -730,8 +730,10 @@ vmxnet3_setup_driver_shared(struct rte_eth_dev *dev) devRead->rssConfDesc.confPA = hw->rss_confPA; } - vmxnet3_dev_vlan_offload_set(dev, - ETH_VLAN_STRIP_MASK | ETH_VLAN_FILTER_MASK); + ret = vmxnet3_dev_vlan_offload_set(dev, + ETH_VLAN_STRIP_MASK | ETH_VLAN_FILTER_MASK); + if (ret) + return ret; vmxnet3_write_mac(hw, dev->data->mac_addrs->addr_bytes); @@ -1279,7 +1281,7 @@ vmxnet3_dev_vlan_filter_set(struct rte_eth_dev *dev, uint16_t vid, int on) return 0; } -static void +static int vmxnet3_dev_vlan_offload_set(struct rte_eth_dev *dev, int mask) { struct vmxnet3_hw *hw = dev->data->dev_private; @@ -1305,6 +1307,8 @@ vmxnet3_dev_vlan_offload_set(struct rte_eth_dev *dev, int mask) VMXNET3_WRITE_BAR1_REG(hw, VMXNET3_REG_CMD, VMXNET3_CMD_UPDATE_VLAN_FILTERS); } + + return 0; } static void diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c index 0b13a58db..94c2fe2f1 100644 --- a/lib/librte_ether/rte_ethdev.c +++ b/lib/librte_ether/rte_ethdev.c @@ -2180,10 +2180,14 @@ rte_eth_dev_set_vlan_offload(uint16_t port_id, int offload_mask) int ret = 0; int mask = 0; int cur, org = 0; + uint64_t orig_offloads; RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV); dev = &rte_eth_devices[port_id]; + /* save original values in case of failure */ + orig_offloads = dev->data->dev_conf.rxmode.offloads; + /*check which option changed by application*/ cur = !!(offload_mask & ETH_VLAN_STRIP_OFFLOAD); org = !!(dev->data->dev_conf.rxmode.offloads & @@ -2236,7 +2240,13 @@ rte_eth_dev_set_vlan_offload(uint16_t port_id, int offload_mask) */ rte_eth_convert_rx_offloads(dev->data->dev_conf.rxmode.offloads, &dev->data->dev_conf.rxmode); - (*dev->dev_ops->vlan_offload_set)(dev, mask); + ret = (*dev->dev_ops->vlan_offload_set)(dev, mask); + if (ret) { + /* hit an error restore original values */ + dev->data->dev_conf.rxmode.offloads = orig_offloads; + rte_eth_convert_rx_offloads(dev->data->dev_conf.rxmode.offloads, + &dev->data->dev_conf.rxmode); + } return ret; } diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h index 2a0129a2a..be74b2af2 100644 --- a/lib/librte_ether/rte_ethdev.h +++ b/lib/librte_ether/rte_ethdev.h @@ -1323,7 +1323,7 @@ typedef int (*vlan_tpid_set_t)(struct rte_eth_dev *dev, enum rte_vlan_type type, uint16_t tpid); /**< @internal set the outer/inner VLAN-TPID by an Ethernet device. */ -typedef void (*vlan_offload_set_t)(struct rte_eth_dev *dev, int mask); +typedef int (*vlan_offload_set_t)(struct rte_eth_dev *dev, int mask); /**< @internal set VLAN offload function by an Ethernet device. */ typedef int (*vlan_pvid_set_t)(struct rte_eth_dev *dev, -- 2.13.6 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [dpdk-dev] [PATCH v5] ethdev: allow returning error on VLAN offload ops 2017-10-25 3:01 ` [dpdk-dev] [PATCH v5] ethdev: allow returning error on VLAN offload ops Ferruh Yigit @ 2017-10-25 22:13 ` Ferruh Yigit 0 siblings, 0 replies; 19+ messages in thread From: Ferruh Yigit @ 2017-10-25 22:13 UTC (permalink / raw) To: John McNamara, Allain Legacy, Matt Peters, Stephen Hurd, Ajit Khaparde, Hemant Agrawal, Shreyansh Jain, Wenzhuo Lu, John Daley, Nelson Escobar, Jing Chen, Jingjing Wu, Beilei Xing, Konstantin Ananyev, Adrien Mazarguil, Nelio Laranjeiro, Yongseok Koh, Alejandro Lucero, Rasesh Mody, Harish Patil, Shahed Shaikh, Yuanhan Liu, Maxime Coquelin, Shrikrishna Khare, Thomas Monjalon Cc: dev, David Harton On 10/24/2017 8:01 PM, Ferruh Yigit wrote: > From: David Harton <dharton@cisco.com> > > Some devices may not support or fail setting VLAN offload > configuration based on dynamic circumstances so the > vlan_offload_set_t vector is modified to return an int so > the caller can determine success or not. > > rte_eth_dev_set_vlan_offload is updated to return the > value provided by the vector when called along with restoring > the original offload configs on failure. > > Existing vlan_offload_set_t vectors are modified to return > an int. Majority of cases return 0 but a few that actually > can fail now return their failure codes. > > Finally, a vlan_offload_set_t vector is added to virtio > to facilitate dynamically turning VLAN strip on or off. > > Signed-off-by: David Harton <dharton@cisco.com> > Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com> Applied to dpdk-next-net/master, thanks. ^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2017-10-25 22:13 UTC | newest] Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-08-24 23:18 [dpdk-dev] [PATCH] ethdev: modifiy vlan_offload_set_t to return int David Harton 2017-08-24 23:37 ` Stephen Hemminger 2017-08-25 0:55 ` David Harton (dharton) 2017-08-25 8:20 ` Bruce Richardson 2017-08-25 13:33 ` [dpdk-dev] [PATCH v2] " David Harton 2017-08-25 13:47 ` [dpdk-dev] [PATCH v3] " David Harton 2017-08-31 22:04 ` Thomas Monjalon 2017-08-31 22:08 ` Thomas Monjalon 2017-09-01 0:40 ` David Harton (dharton) 2017-09-01 8:01 ` Thomas Monjalon 2017-09-01 2:36 ` [dpdk-dev] [PATCH v4] ethdev: allow returning error on VLAN offload configuration David Harton 2017-09-01 7:41 ` Hemant Agrawal 2017-09-01 12:54 ` David Harton (dharton) 2017-09-07 9:37 ` Hemant Agrawal 2017-09-07 12:09 ` David Harton (dharton) 2017-10-10 5:34 ` Ferruh Yigit 2017-10-10 12:20 ` David Harton (dharton) 2017-10-25 3:01 ` [dpdk-dev] [PATCH v5] ethdev: allow returning error on VLAN offload ops Ferruh Yigit 2017-10-25 22:13 ` Ferruh Yigit
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).