All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] testpmd: check return value of rte_eth_dev_vlan_filter()
@ 2015-01-23 10:43 Michal Jastrzebski
       [not found] ` <1422009831-11092-1-git-send-email-michalx.k.jastrzebski-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
  0 siblings, 1 reply; 4+ messages in thread
From: Michal Jastrzebski @ 2015-01-23 10:43 UTC (permalink / raw)
  To: dev-VfR2kkLFssw

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.

Signed-off by: Michal Jastrzebski <michalx.k.jastrzebski-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
 app/test-pmd/config.c         |   14 ++++++++------
 app/test-pmd/testpmd.h        |    2 +-
 lib/librte_ether/rte_ethdev.c |    4 ++--
 3 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 97b6525..6ef6cb0 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,9 @@ 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 f8b0740..ee03a07 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -489,7 +489,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 95f2ceb..2e720f7 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -1478,8 +1478,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

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] testpmd: check return value of rte_eth_dev_vlan_filter()
       [not found] ` <1422009831-11092-1-git-send-email-michalx.k.jastrzebski-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
@ 2015-01-27 10:33   ` Thomas Monjalon
  2015-01-27 15:58     ` Jastrzebski, MichalX K
  0 siblings, 1 reply; 4+ messages in thread
From: Thomas Monjalon @ 2015-01-27 10:33 UTC (permalink / raw)
  To: Michal Jastrzebski; +Cc: dev-VfR2kkLFssw

Hi Michal,

2015-01-23 11:43, 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.
> 
> Signed-off by: Michal Jastrzebski <michalx.k.jastrzebski-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

checkpatch is not happy because you forgot an hyphen.

> @@ -1667,8 +1668,9 @@ 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;

Again, checkpatch does not like this line.

And more importantly, you make it clear that sometimes we cannot enable all
vlans and return no error.
So I wonder how is it documented in the testpmd help?

Thanks
-- 
Thomas

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] testpmd: check return value of rte_eth_dev_vlan_filter()
  2015-01-27 10:33   ` Thomas Monjalon
@ 2015-01-27 15:58     ` Jastrzebski, MichalX K
       [not found]       ` <60ABE07DBB3A454EB7FAD707B4BB1582138D57F7-kPTMFJFq+rHjxeytcECX8bfspsVTdybXVpNB7YpNyf8@public.gmane.org>
  0 siblings, 1 reply; 4+ messages in thread
From: Jastrzebski, MichalX K @ 2015-01-27 15:58 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev-VfR2kkLFssw

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon-pdR9zngts4EAvxtiuMwx3w@public.gmane.org]
> Sent: Tuesday, January 27, 2015 11:33 AM
> To: Jastrzebski, MichalX K
> Cc: dev-VfR2kkLFssw@public.gmane.org
> Subject: Re: [dpdk-dev] [PATCH] testpmd: check return value of
> rte_eth_dev_vlan_filter()
> 
> Hi Michal,
> 
> 2015-01-23 11:43, 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.
> >
> > Signed-off by: Michal Jastrzebski <michalx.k.jastrzebski-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> 
> checkpatch is not happy because you forgot an hyphen.
> 
> > @@ -1667,8 +1668,9 @@ 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;
> 
> Again, checkpatch does not like this line.
> 
Hi Thomas,
Thanks for pointed it out. I have already fixed all checkpatch.pl errors.
I will send v2 patch for this,
> And more importantly, you make it clear that sometimes we cannot enable
> all
> vlans and return no error.
Should I return this error somewhere? Isn't just printing the error best option here?
> So I wonder how is it documented in the testpmd help?
I can add a note in testpmd_funcs.rst file or I can place some info in .help_str?
What do you mean "testpmd help"?
> 
> Thanks
> --
> Thomas

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] testpmd: check return value of rte_eth_dev_vlan_filter()
       [not found]       ` <60ABE07DBB3A454EB7FAD707B4BB1582138D57F7-kPTMFJFq+rHjxeytcECX8bfspsVTdybXVpNB7YpNyf8@public.gmane.org>
@ 2015-01-27 22:22         ` Thomas Monjalon
  0 siblings, 0 replies; 4+ messages in thread
From: Thomas Monjalon @ 2015-01-27 22:22 UTC (permalink / raw)
  To: Jastrzebski, MichalX K; +Cc: dev-VfR2kkLFssw

2015-01-27 15:58, Jastrzebski, MichalX K:
> From: Thomas Monjalon [mailto:thomas.monjalon-pdR9zngts4EAvxtiuMwx3w@public.gmane.org]
> > And more importantly, you make it clear that sometimes we cannot enable
> > all vlans and return no error.
> 
> Should I return this error somewhere? Isn't just printing the error best option here?

Yes printing a warning before "break" seems a good idea.

> > So I wonder how is it documented in the testpmd help?
> 
> I can add a note in testpmd_funcs.rst file or I can place some info in .help_str?
> What do you mean "testpmd help"?

I mean both :)
But maybe it's not appropriate in .help_str, I'm not sure.

-- 
Thomas

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2015-01-27 22:22 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-01-23 10:43 [PATCH] testpmd: check return value of rte_eth_dev_vlan_filter() Michal Jastrzebski
     [not found] ` <1422009831-11092-1-git-send-email-michalx.k.jastrzebski-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
2015-01-27 10:33   ` Thomas Monjalon
2015-01-27 15:58     ` Jastrzebski, MichalX K
     [not found]       ` <60ABE07DBB3A454EB7FAD707B4BB1582138D57F7-kPTMFJFq+rHjxeytcECX8bfspsVTdybXVpNB7YpNyf8@public.gmane.org>
2015-01-27 22:22         ` Thomas Monjalon

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.