> -----Original Message----- > From: Qiu, Michael > Sent: Wednesday, January 28, 2015 2:38 AM > To: Jastrzebski, MichalX K; dev at dpdk.org > Subject: Re: [dpdk-dev] [PATCH v2] testpmd check return value of > rte_eth_dev_vlan_filter() > > On 1/28/2015 1:20 AM, Michal Jastrzebski wrote: > > This patch modifies testpmd behavior when setting: > > rx_vlan add all vf_port (enabling all vlanids > > to be passed thru rx filter on VF). > > Rx_vlan_all_filter_set() function, > > checks if the next vlanid can be enabled by the driver. > > Number of vlanids is limited by the NIC and thus the NIC > > do not allow to enable more vlanids than it can allocate > > in VFTA table. > > But what about if it is caused by other issue to lead a enable failure?
Hi Michael, I am sorry I am writing almost a month after you reply But I had another issues required a 100% attention. As I see in the function: rte_eth_dev_vlan_filter(uint8_t port_id, uint16_t vlan_id, int on); arguments like port_id and vlan_id are checked. Also if vlan-filtering is enabled is checked. Each error is printed so the user is informed that something went wrong and where to look for it. I don't know what else could be checked here? > > > > v2 - fix formatting errors > > > > Signed-off-by: Michal Jastrzebski <michalx.k.jastrzebski at intel.com> > > --- > > app/test-pmd/config.c | 15 +++++++++------ > > app/test-pmd/testpmd.h | 2 +- > > lib/librte_ether/rte_ethdev.c | 4 ++-- > > 3 files changed, 12 insertions(+), 9 deletions(-) > > > > diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c > > index c40f819..eda737e 100644 > > --- a/app/test-pmd/config.c > > +++ b/app/test-pmd/config.c > > @@ -1643,21 +1643,22 @@ rx_vlan_filter_set(portid_t port_id, int on) > > "diag=%d\n", port_id, on, diag); > > } > > > > -void > > +int > > rx_vft_set(portid_t port_id, uint16_t vlan_id, int on) > > { > > int diag; > > > > if (port_id_is_invalid(port_id)) > > - return; > > + return 1; > > if (vlan_id_is_invalid(vlan_id)) > > - return; > > + return 1; > > diag = rte_eth_dev_vlan_filter(port_id, vlan_id, on); > > if (diag == 0) > > - return; > > + return 0; > > printf("rte_eth_dev_vlan_filter(port_pi=%d, vlan_id=%d, on=%d) > failed " > > "diag=%d\n", > > port_id, vlan_id, on, diag); > > + return -1; > > } > > > > void > > @@ -1667,8 +1668,10 @@ rx_vlan_all_filter_set(portid_t port_id, int on) > > > > if (port_id_is_invalid(port_id)) > > return; > > - for (vlan_id = 0; vlan_id < 4096; vlan_id++) > > - rx_vft_set(port_id, vlan_id, on); > > + for (vlan_id = 0; vlan_id < 4096; vlan_id++) { > > Before "{" you use a Tab? One white space is OK. > > Thanks, > Michael > > > + if (rx_vft_set(port_id, vlan_id, on)) > > + break; > > + } > > } > > > > void > > diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h > > index 8f5e6c7..e0186b9 100644 > > --- a/app/test-pmd/testpmd.h > > +++ b/app/test-pmd/testpmd.h > > @@ -492,7 +492,7 @@ void rx_vlan_strip_set_on_queue(portid_t port_id, > uint16_t queue_id, int on); > > > > void rx_vlan_filter_set(portid_t port_id, int on); > > void rx_vlan_all_filter_set(portid_t port_id, int on); > > -void rx_vft_set(portid_t port_id, uint16_t vlan_id, int on); > > +int rx_vft_set(portid_t port_id, uint16_t vlan_id, int on); > > void vlan_extend_set(portid_t port_id, int on); > > void vlan_tpid_set(portid_t port_id, uint16_t tp_id); > > void tx_vlan_set(portid_t port_id, uint16_t vlan_id); > > diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c > > index ea3a1fb..064b5d6 100644 > > --- a/lib/librte_ether/rte_ethdev.c > > +++ b/lib/librte_ether/rte_ethdev.c > > @@ -1519,8 +1519,8 @@ rte_eth_dev_vlan_filter(uint8_t port_id, > uint16_t vlan_id, int on) > > return (-EINVAL); > > } > > FUNC_PTR_OR_ERR_RET(*dev->dev_ops->vlan_filter_set, - > ENOTSUP); > > - (*dev->dev_ops->vlan_filter_set)(dev, vlan_id, on); > > - return (0); > > + > > + return (*dev->dev_ops->vlan_filter_set)(dev, vlan_id, on); > > } > > > > int