[dpdk-dev] [PATCH v2] testpmd check return value of rte_eth_dev_vlan_filter()

2015-02-20 Thread Jastrzebski, MichalX K
> -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 
> > ---
> >  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



[dpdk-dev] [PATCH v2] testpmd check return value of rte_eth_dev_vlan_filter()

2015-01-28 Thread Qiu, Michael
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?


> v2 - fix formatting errors
>
> Signed-off-by: Michal Jastrzebski 
> ---
>  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



[dpdk-dev] [PATCH v2] testpmd check return value of rte_eth_dev_vlan_filter()

2015-01-27 Thread Michal Jastrzebski
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.

v2 - fix formatting errors

Signed-off-by: Michal Jastrzebski 
---
 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++){
+   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
-- 
1.7.9.5