DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH v2 0/2] lib/librte_pmd_i40e: set vlan filter fix
@ 2014-11-10  2:46 Huawei Xie
  2014-11-10  2:46 ` [dpdk-dev] [PATCH v2 1/2] " Huawei Xie
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Huawei Xie @ 2014-11-10  2:46 UTC (permalink / raw)
  To: dev

This patchset fixes "set vlan filter" issue.

v2 changes:
* add two macros I40E_VFTA_IDX and I40E_VFTA_BIT for VFTA array operation.

Huawei Xie (2):
  vlan id set fix
  add I40E_VFTA_IDX and I40E_VFTA_BIT macros for VFTA related operation

 lib/librte_pmd_i40e/i40e_ethdev.c | 20 ++++++++++----------
 lib/librte_pmd_i40e/i40e_ethdev.h |  9 +++++++++
 2 files changed, 19 insertions(+), 10 deletions(-)

-- 
1.8.1.4

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

* [dpdk-dev] [PATCH v2 1/2] lib/librte_pmd_i40e: set vlan filter fix
  2014-11-10  2:46 [dpdk-dev] [PATCH v2 0/2] lib/librte_pmd_i40e: set vlan filter fix Huawei Xie
@ 2014-11-10  2:46 ` Huawei Xie
  2014-11-10  5:08   ` Zhang, Helin
  2014-11-10  2:46 ` [dpdk-dev] [PATCH v2 2/2] lib/librte_pmd_i40e: add I40E_VFTA_IDX and I40E_VFTA_BIT macros for VFTA related operation Huawei Xie
  2014-11-10  8:25 ` [dpdk-dev] [PATCH v2 0/2] lib/librte_pmd_i40e: set vlan filter fix Chen, Jing D
  2 siblings, 1 reply; 11+ messages in thread
From: Huawei Xie @ 2014-11-10  2:46 UTC (permalink / raw)
  To: dev

">> 5" rather than ">> 4"

Signed-off-by: Huawei Xie <huawei.xie@intel.com>
---
 lib/librte_pmd_i40e/i40e_ethdev.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/lib/librte_pmd_i40e/i40e_ethdev.c b/lib/librte_pmd_i40e/i40e_ethdev.c
index 5074262..c0cf3cf 100644
--- a/lib/librte_pmd_i40e/i40e_ethdev.c
+++ b/lib/librte_pmd_i40e/i40e_ethdev.c
@@ -4048,14 +4048,11 @@ i40e_set_vlan_filter(struct i40e_vsi *vsi,
 {
 	uint32_t vid_idx, vid_bit;
 
-#define UINT32_BIT_MASK      0x1F
-#define VALID_VLAN_BIT_MASK  0xFFF
 	/* VFTA is 32-bits size array, each element contains 32 vlan bits, Find the
 	 *  element first, then find the bits it belongs to
 	 */
-	vid_idx = (uint32_t) ((vlan_id & VALID_VLAN_BIT_MASK) >>
-		  sizeof(uint32_t));
-	vid_bit = (uint32_t) (1 << (vlan_id & UINT32_BIT_MASK));
+	vid_idx = (uint32_t) ((vlan_id >> 5 ) & 0x7F);
+	vid_bit = (uint32_t) (1 << (vlan_id & 0x1F));
 
 	if (on)
 		vsi->vfta[vid_idx] |= vid_bit;
-- 
1.8.1.4

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

* [dpdk-dev] [PATCH v2 2/2] lib/librte_pmd_i40e: add I40E_VFTA_IDX and I40E_VFTA_BIT macros for VFTA related operation
  2014-11-10  2:46 [dpdk-dev] [PATCH v2 0/2] lib/librte_pmd_i40e: set vlan filter fix Huawei Xie
  2014-11-10  2:46 ` [dpdk-dev] [PATCH v2 1/2] " Huawei Xie
@ 2014-11-10  2:46 ` Huawei Xie
  2014-11-10  5:08   ` Zhang, Helin
  2014-11-10  8:25 ` [dpdk-dev] [PATCH v2 0/2] lib/librte_pmd_i40e: set vlan filter fix Chen, Jing D
  2 siblings, 1 reply; 11+ messages in thread
From: Huawei Xie @ 2014-11-10  2:46 UTC (permalink / raw)
  To: dev

Add two macros I40E_VFTA_IDX and I40E_VFTA_BIT for VFTA manipulation.
Add vlan_id check in vlan filter search and set function.

Signed-off-by: Huawei Xie <huawei.xie@intel.com>
---
 lib/librte_pmd_i40e/i40e_ethdev.c | 17 ++++++++++-------
 lib/librte_pmd_i40e/i40e_ethdev.h |  9 +++++++++
 2 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/lib/librte_pmd_i40e/i40e_ethdev.c b/lib/librte_pmd_i40e/i40e_ethdev.c
index c0cf3cf..245460f 100644
--- a/lib/librte_pmd_i40e/i40e_ethdev.c
+++ b/lib/librte_pmd_i40e/i40e_ethdev.c
@@ -4033,8 +4033,11 @@ i40e_find_vlan_filter(struct i40e_vsi *vsi,
 {
 	uint32_t vid_idx, vid_bit;
 
-	vid_idx = (uint32_t) ((vlan_id >> 5) & 0x7F);
-	vid_bit = (uint32_t) (1 << (vlan_id & 0x1F));
+	if (vlan_id > ETH_VLAN_ID_MAX)
+		return 0;
+
+	vid_idx = I40E_VFTA_IDX(vlan_id);
+	vid_bit = I40E_VFTA_BIT(vlan_id);
 
 	if (vsi->vfta[vid_idx] & vid_bit)
 		return 1;
@@ -4048,11 +4051,11 @@ i40e_set_vlan_filter(struct i40e_vsi *vsi,
 {
 	uint32_t vid_idx, vid_bit;
 
-	/* VFTA is 32-bits size array, each element contains 32 vlan bits, Find the
-	 *  element first, then find the bits it belongs to
-	 */
-	vid_idx = (uint32_t) ((vlan_id >> 5 ) & 0x7F);
-	vid_bit = (uint32_t) (1 << (vlan_id & 0x1F));
+	if (vlan_id > ETH_VLAN_ID_MAX)
+		return;
+
+	vid_idx = I40E_VFTA_IDX(vlan_id);
+	vid_bit = I40E_VFTA_BIT(vlan_id);
 
 	if (on)
 		vsi->vfta[vid_idx] |= vid_bit;
diff --git a/lib/librte_pmd_i40e/i40e_ethdev.h b/lib/librte_pmd_i40e/i40e_ethdev.h
index 96361c2..4f2c16a 100644
--- a/lib/librte_pmd_i40e/i40e_ethdev.h
+++ b/lib/librte_pmd_i40e/i40e_ethdev.h
@@ -50,6 +50,15 @@
 #define I40E_DEFAULT_QP_NUM_FDIR  64
 #define I40E_UINT32_BIT_SIZE      (CHAR_BIT * sizeof(uint32_t))
 #define I40E_VFTA_SIZE            (4096 / I40E_UINT32_BIT_SIZE)
+/*
+ * vlan_id is a 12 bit number.
+ * The VFTA array is actually a 4096 bit array, 128 of 32bit elements.
+ * 2^5 = 32. The val of lower 5 bits specifies the bit in the 32bit element.
+ * The higher 7 bit val specifies VFTA array index.
+ */
+#define I40E_VFTA_BIT(vlan_id)    (1 << ((vlan_id) & 0x1F))
+#define I40E_VFTA_IDX(vlan_id)    ((vlan_id) >> 5)
+
 /* Default TC traffic in case DCB is not enabled */
 #define I40E_DEFAULT_TCMAP        0x1
 
-- 
1.8.1.4

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

* Re: [dpdk-dev] [PATCH v2 2/2] lib/librte_pmd_i40e: add I40E_VFTA_IDX and I40E_VFTA_BIT macros for VFTA related operation
  2014-11-10  2:46 ` [dpdk-dev] [PATCH v2 2/2] lib/librte_pmd_i40e: add I40E_VFTA_IDX and I40E_VFTA_BIT macros for VFTA related operation Huawei Xie
@ 2014-11-10  5:08   ` Zhang, Helin
  2014-11-10  6:52     ` Xie, Huawei
  0 siblings, 1 reply; 11+ messages in thread
From: Zhang, Helin @ 2014-11-10  5:08 UTC (permalink / raw)
  To: Xie, Huawei, dev

Hi Huawei

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Huawei Xie
> Sent: Monday, November 10, 2014 10:46 AM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH v2 2/2] lib/librte_pmd_i40e: add I40E_VFTA_IDX and
> I40E_VFTA_BIT macros for VFTA related operation
> 
> Add two macros I40E_VFTA_IDX and I40E_VFTA_BIT for VFTA manipulation.
> Add vlan_id check in vlan filter search and set function.
> 
> Signed-off-by: Huawei Xie <huawei.xie@intel.com>
> ---
>  lib/librte_pmd_i40e/i40e_ethdev.c | 17 ++++++++++-------
> lib/librte_pmd_i40e/i40e_ethdev.h |  9 +++++++++
>  2 files changed, 19 insertions(+), 7 deletions(-)
> 
> diff --git a/lib/librte_pmd_i40e/i40e_ethdev.c
> b/lib/librte_pmd_i40e/i40e_ethdev.c
> index c0cf3cf..245460f 100644
> --- a/lib/librte_pmd_i40e/i40e_ethdev.c
> +++ b/lib/librte_pmd_i40e/i40e_ethdev.c
> @@ -4033,8 +4033,11 @@ i40e_find_vlan_filter(struct i40e_vsi *vsi,  {
>  	uint32_t vid_idx, vid_bit;
> 
> -	vid_idx = (uint32_t) ((vlan_id >> 5) & 0x7F);
> -	vid_bit = (uint32_t) (1 << (vlan_id & 0x1F));
> +	if (vlan_id > ETH_VLAN_ID_MAX)
> +		return 0;
> +
> +	vid_idx = I40E_VFTA_IDX(vlan_id);
> +	vid_bit = I40E_VFTA_BIT(vlan_id);
> 
>  	if (vsi->vfta[vid_idx] & vid_bit)
>  		return 1;
> @@ -4048,11 +4051,11 @@ i40e_set_vlan_filter(struct i40e_vsi *vsi,  {
>  	uint32_t vid_idx, vid_bit;
> 
> -	/* VFTA is 32-bits size array, each element contains 32 vlan bits, Find the
> -	 *  element first, then find the bits it belongs to
> -	 */
> -	vid_idx = (uint32_t) ((vlan_id >> 5 ) & 0x7F);
> -	vid_bit = (uint32_t) (1 << (vlan_id & 0x1F));
> +	if (vlan_id > ETH_VLAN_ID_MAX)
> +		return;
> +
> +	vid_idx = I40E_VFTA_IDX(vlan_id);
> +	vid_bit = I40E_VFTA_BIT(vlan_id);
> 
>  	if (on)
>  		vsi->vfta[vid_idx] |= vid_bit;
> diff --git a/lib/librte_pmd_i40e/i40e_ethdev.h
> b/lib/librte_pmd_i40e/i40e_ethdev.h
> index 96361c2..4f2c16a 100644
> --- a/lib/librte_pmd_i40e/i40e_ethdev.h
> +++ b/lib/librte_pmd_i40e/i40e_ethdev.h
> @@ -50,6 +50,15 @@
>  #define I40E_DEFAULT_QP_NUM_FDIR  64
>  #define I40E_UINT32_BIT_SIZE      (CHAR_BIT * sizeof(uint32_t))
>  #define I40E_VFTA_SIZE            (4096 / I40E_UINT32_BIT_SIZE)
> +/*
> + * vlan_id is a 12 bit number.
> + * The VFTA array is actually a 4096 bit array, 128 of 32bit elements.
> + * 2^5 = 32. The val of lower 5 bits specifies the bit in the 32bit element.
> + * The higher 7 bit val specifies VFTA array index.
> + */
> +#define I40E_VFTA_BIT(vlan_id)    (1 << ((vlan_id) & 0x1F))
> +#define I40E_VFTA_IDX(vlan_id)    ((vlan_id) >> 5)
Why not define the 0x1f and 5 more meaningful in macros?

Why define it in this header file? It seems that only used in i40e_ethdev.c.

> +
>  /* Default TC traffic in case DCB is not enabled */
>  #define I40E_DEFAULT_TCMAP        0x1
> 
> --
> 1.8.1.4

Regards,
Helin

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

* Re: [dpdk-dev] [PATCH v2 1/2] lib/librte_pmd_i40e: set vlan filter fix
  2014-11-10  2:46 ` [dpdk-dev] [PATCH v2 1/2] " Huawei Xie
@ 2014-11-10  5:08   ` Zhang, Helin
  2014-11-10  6:42     ` Xie, Huawei
  0 siblings, 1 reply; 11+ messages in thread
From: Zhang, Helin @ 2014-11-10  5:08 UTC (permalink / raw)
  To: Xie, Huawei, dev



> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Huawei Xie
> Sent: Monday, November 10, 2014 10:46 AM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH v2 1/2] lib/librte_pmd_i40e: set vlan filter fix
> 
> ">> 5" rather than ">> 4"
> 
> Signed-off-by: Huawei Xie <huawei.xie@intel.com>
> ---
>  lib/librte_pmd_i40e/i40e_ethdev.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/lib/librte_pmd_i40e/i40e_ethdev.c
> b/lib/librte_pmd_i40e/i40e_ethdev.c
> index 5074262..c0cf3cf 100644
> --- a/lib/librte_pmd_i40e/i40e_ethdev.c
> +++ b/lib/librte_pmd_i40e/i40e_ethdev.c
> @@ -4048,14 +4048,11 @@ i40e_set_vlan_filter(struct i40e_vsi *vsi,  {
>  	uint32_t vid_idx, vid_bit;
> 
> -#define UINT32_BIT_MASK      0x1F
> -#define VALID_VLAN_BIT_MASK  0xFFF
>  	/* VFTA is 32-bits size array, each element contains 32 vlan bits, Find the
>  	 *  element first, then find the bits it belongs to
>  	 */
> -	vid_idx = (uint32_t) ((vlan_id & VALID_VLAN_BIT_MASK) >>
> -		  sizeof(uint32_t));
> -	vid_bit = (uint32_t) (1 << (vlan_id & UINT32_BIT_MASK));
> +	vid_idx = (uint32_t) ((vlan_id >> 5 ) & 0x7F);
> +	vid_bit = (uint32_t) (1 << (vlan_id & 0x1F));
I don't understand why remove macros and use numeric instead?

> 
>  	if (on)
>  		vsi->vfta[vid_idx] |= vid_bit;
> --
> 1.8.1.4

Regards,
Helin

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

* Re: [dpdk-dev] [PATCH v2 1/2] lib/librte_pmd_i40e: set vlan filter fix
  2014-11-10  5:08   ` Zhang, Helin
@ 2014-11-10  6:42     ` Xie, Huawei
  2014-11-24  8:32       ` Qiu, Michael
  0 siblings, 1 reply; 11+ messages in thread
From: Xie, Huawei @ 2014-11-10  6:42 UTC (permalink / raw)
  To: Zhang, Helin, dev



> -----Original Message-----
> From: Zhang, Helin
> Sent: Sunday, November 09, 2014 10:09 PM
> To: Xie, Huawei; dev@dpdk.org
> Subject: RE: [dpdk-dev] [PATCH v2 1/2] lib/librte_pmd_i40e: set vlan filter fix
> 
> 
> 
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Huawei Xie
> > Sent: Monday, November 10, 2014 10:46 AM
> > To: dev@dpdk.org
> > Subject: [dpdk-dev] [PATCH v2 1/2] lib/librte_pmd_i40e: set vlan filter fix
> >
> > ">> 5" rather than ">> 4"
> >
> > Signed-off-by: Huawei Xie <huawei.xie@intel.com>
> > ---
> >  lib/librte_pmd_i40e/i40e_ethdev.c | 7 ++-----
> >  1 file changed, 2 insertions(+), 5 deletions(-)
> >
> > diff --git a/lib/librte_pmd_i40e/i40e_ethdev.c
> > b/lib/librte_pmd_i40e/i40e_ethdev.c
> > index 5074262..c0cf3cf 100644
> > --- a/lib/librte_pmd_i40e/i40e_ethdev.c
> > +++ b/lib/librte_pmd_i40e/i40e_ethdev.c
> > @@ -4048,14 +4048,11 @@ i40e_set_vlan_filter(struct i40e_vsi *vsi,  {
> >  	uint32_t vid_idx, vid_bit;
> >
> > -#define UINT32_BIT_MASK      0x1F
> > -#define VALID_VLAN_BIT_MASK  0xFFF
> >  	/* VFTA is 32-bits size array, each element contains 32 vlan bits, Find the
> >  	 *  element first, then find the bits it belongs to
> >  	 */
> > -	vid_idx = (uint32_t) ((vlan_id & VALID_VLAN_BIT_MASK) >>
> > -		  sizeof(uint32_t));
> > -	vid_bit = (uint32_t) (1 << (vlan_id & UINT32_BIT_MASK));
> > +	vid_idx = (uint32_t) ((vlan_id >> 5 ) & 0x7F);
> > +	vid_bit = (uint32_t) (1 << (vlan_id & 0x1F));
> I don't understand why remove macros and use numeric instead?
Those macros are wrongly defined. Correct macros are defined in second patch.
> 
> >
> >  	if (on)
> >  		vsi->vfta[vid_idx] |= vid_bit;
> > --
> > 1.8.1.4
> 
> Regards,
> Helin

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

* Re: [dpdk-dev] [PATCH v2 2/2] lib/librte_pmd_i40e: add I40E_VFTA_IDX and I40E_VFTA_BIT macros for VFTA related operation
  2014-11-10  5:08   ` Zhang, Helin
@ 2014-11-10  6:52     ` Xie, Huawei
  0 siblings, 0 replies; 11+ messages in thread
From: Xie, Huawei @ 2014-11-10  6:52 UTC (permalink / raw)
  To: Zhang, Helin, dev



> -----Original Message-----
> From: Zhang, Helin
> Sent: Sunday, November 09, 2014 10:08 PM
> To: Xie, Huawei; dev@dpdk.org
> Subject: RE: [dpdk-dev] [PATCH v2 2/2] lib/librte_pmd_i40e: add I40E_VFTA_IDX
> and I40E_VFTA_BIT macros for VFTA related operation
> 
> Hi Huawei
> 
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Huawei Xie
> > Sent: Monday, November 10, 2014 10:46 AM
> > To: dev@dpdk.org
> > Subject: [dpdk-dev] [PATCH v2 2/2] lib/librte_pmd_i40e: add I40E_VFTA_IDX
> and
> > I40E_VFTA_BIT macros for VFTA related operation
> >
> > Add two macros I40E_VFTA_IDX and I40E_VFTA_BIT for VFTA manipulation.
> > Add vlan_id check in vlan filter search and set function.
> >
> > Signed-off-by: Huawei Xie <huawei.xie@intel.com>
> > ---
> >  lib/librte_pmd_i40e/i40e_ethdev.c | 17 ++++++++++-------
> > lib/librte_pmd_i40e/i40e_ethdev.h |  9 +++++++++
> >  2 files changed, 19 insertions(+), 7 deletions(-)
> >
> > diff --git a/lib/librte_pmd_i40e/i40e_ethdev.c
> > b/lib/librte_pmd_i40e/i40e_ethdev.c
> > index c0cf3cf..245460f 100644
> > --- a/lib/librte_pmd_i40e/i40e_ethdev.c
> > +++ b/lib/librte_pmd_i40e/i40e_ethdev.c
> > @@ -4033,8 +4033,11 @@ i40e_find_vlan_filter(struct i40e_vsi *vsi,  {
> >  	uint32_t vid_idx, vid_bit;
> >
> > -	vid_idx = (uint32_t) ((vlan_id >> 5) & 0x7F);
> > -	vid_bit = (uint32_t) (1 << (vlan_id & 0x1F));
> > +	if (vlan_id > ETH_VLAN_ID_MAX)
> > +		return 0;
> > +
> > +	vid_idx = I40E_VFTA_IDX(vlan_id);
> > +	vid_bit = I40E_VFTA_BIT(vlan_id);
> >
> >  	if (vsi->vfta[vid_idx] & vid_bit)
> >  		return 1;
> > @@ -4048,11 +4051,11 @@ i40e_set_vlan_filter(struct i40e_vsi *vsi,  {
> >  	uint32_t vid_idx, vid_bit;
> >
> > -	/* VFTA is 32-bits size array, each element contains 32 vlan bits, Find the
> > -	 *  element first, then find the bits it belongs to
> > -	 */
> > -	vid_idx = (uint32_t) ((vlan_id >> 5 ) & 0x7F);
> > -	vid_bit = (uint32_t) (1 << (vlan_id & 0x1F));
> > +	if (vlan_id > ETH_VLAN_ID_MAX)
> > +		return;
> > +
> > +	vid_idx = I40E_VFTA_IDX(vlan_id);
> > +	vid_bit = I40E_VFTA_BIT(vlan_id);
> >
> >  	if (on)
> >  		vsi->vfta[vid_idx] |= vid_bit;
> > diff --git a/lib/librte_pmd_i40e/i40e_ethdev.h
> > b/lib/librte_pmd_i40e/i40e_ethdev.h
> > index 96361c2..4f2c16a 100644
> > --- a/lib/librte_pmd_i40e/i40e_ethdev.h
> > +++ b/lib/librte_pmd_i40e/i40e_ethdev.h
> > @@ -50,6 +50,15 @@
> >  #define I40E_DEFAULT_QP_NUM_FDIR  64
> >  #define I40E_UINT32_BIT_SIZE      (CHAR_BIT * sizeof(uint32_t))
> >  #define I40E_VFTA_SIZE            (4096 / I40E_UINT32_BIT_SIZE)
> > +/*
> > + * vlan_id is a 12 bit number.
> > + * The VFTA array is actually a 4096 bit array, 128 of 32bit elements.
> > + * 2^5 = 32. The val of lower 5 bits specifies the bit in the 32bit element.
> > + * The higher 7 bit val specifies VFTA array index.
> > + */
> > +#define I40E_VFTA_BIT(vlan_id)    (1 << ((vlan_id) & 0x1F))
> > +#define I40E_VFTA_IDX(vlan_id)    ((vlan_id) >> 5)
> Why not define the 0x1f and 5 more meaningful in macros?
> 
> Why define it in this header file? It seems that only used in i40e_ethdev.c.
> 
It is a macro for i40e common functionality, as I40E_VFTA_SIZE macro.
> > +
> >  /* Default TC traffic in case DCB is not enabled */
> >  #define I40E_DEFAULT_TCMAP        0x1
> >
> > --
> > 1.8.1.4
> 
> Regards,
> Helin

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

* Re: [dpdk-dev] [PATCH v2 0/2] lib/librte_pmd_i40e: set vlan filter fix
  2014-11-10  2:46 [dpdk-dev] [PATCH v2 0/2] lib/librte_pmd_i40e: set vlan filter fix Huawei Xie
  2014-11-10  2:46 ` [dpdk-dev] [PATCH v2 1/2] " Huawei Xie
  2014-11-10  2:46 ` [dpdk-dev] [PATCH v2 2/2] lib/librte_pmd_i40e: add I40E_VFTA_IDX and I40E_VFTA_BIT macros for VFTA related operation Huawei Xie
@ 2014-11-10  8:25 ` Chen, Jing D
  2014-11-21 20:47   ` Xie, Huawei
  2 siblings, 1 reply; 11+ messages in thread
From: Chen, Jing D @ 2014-11-10  8:25 UTC (permalink / raw)
  To: Xie, Huawei, dev



> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Huawei Xie
> Sent: Monday, November 10, 2014 10:46 AM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH v2 0/2] lib/librte_pmd_i40e: set vlan filter fix
> 
> This patchset fixes "set vlan filter" issue.
> 
> v2 changes:
> * add two macros I40E_VFTA_IDX and I40E_VFTA_BIT for VFTA array
> operation.
> 
> Huawei Xie (2):
>   vlan id set fix
>   add I40E_VFTA_IDX and I40E_VFTA_BIT macros for VFTA related operation
> 
>  lib/librte_pmd_i40e/i40e_ethdev.c | 20 ++++++++++----------
>  lib/librte_pmd_i40e/i40e_ethdev.h |  9 +++++++++
>  2 files changed, 19 insertions(+), 10 deletions(-)
> 
> --
> 1.8.1.4

Acked-by : Jing Chen <jing.d.chen@intel.com>

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

* Re: [dpdk-dev] [PATCH v2 0/2] lib/librte_pmd_i40e: set vlan filter fix
  2014-11-10  8:25 ` [dpdk-dev] [PATCH v2 0/2] lib/librte_pmd_i40e: set vlan filter fix Chen, Jing D
@ 2014-11-21 20:47   ` Xie, Huawei
  0 siblings, 0 replies; 11+ messages in thread
From: Xie, Huawei @ 2014-11-21 20:47 UTC (permalink / raw)
  To: Thomas Monjalon (thomas.monjalon, Chen, Jing D, dev

Hi Thomas:
Do you have issues applying this patch and vhost VMDQ patch?
Without this fix,  the vlan filter set willn't work.

-Huawei

> -----Original Message-----
> From: Chen, Jing D
> Sent: Monday, November 10, 2014 1:26 AM
> To: Xie, Huawei; dev@dpdk.org
> Subject: RE: [dpdk-dev] [PATCH v2 0/2] lib/librte_pmd_i40e: set vlan filter fix
> 
> 
> 
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Huawei Xie
> > Sent: Monday, November 10, 2014 10:46 AM
> > To: dev@dpdk.org
> > Subject: [dpdk-dev] [PATCH v2 0/2] lib/librte_pmd_i40e: set vlan filter fix
> >
> > This patchset fixes "set vlan filter" issue.
> >
> > v2 changes:
> > * add two macros I40E_VFTA_IDX and I40E_VFTA_BIT for VFTA array
> > operation.
> >
> > Huawei Xie (2):
> >   vlan id set fix
> >   add I40E_VFTA_IDX and I40E_VFTA_BIT macros for VFTA related operation
> >
> >  lib/librte_pmd_i40e/i40e_ethdev.c | 20 ++++++++++----------
> >  lib/librte_pmd_i40e/i40e_ethdev.h |  9 +++++++++
> >  2 files changed, 19 insertions(+), 10 deletions(-)
> >
> > --
> > 1.8.1.4
> 
> Acked-by : Jing Chen <jing.d.chen@intel.com>

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

* Re: [dpdk-dev] [PATCH v2 1/2] lib/librte_pmd_i40e: set vlan filter fix
  2014-11-10  6:42     ` Xie, Huawei
@ 2014-11-24  8:32       ` Qiu, Michael
  2014-11-24 21:41         ` Xie, Huawei
  0 siblings, 1 reply; 11+ messages in thread
From: Qiu, Michael @ 2014-11-24  8:32 UTC (permalink / raw)
  To: Xie, Huawei, Zhang, Helin, dev

On 11/10/2014 2:42 PM, Xie, Huawei wrote:
>
>> -----Original Message-----
>> From: Zhang, Helin
>> Sent: Sunday, November 09, 2014 10:09 PM
>> To: Xie, Huawei; dev@dpdk.org
>> Subject: RE: [dpdk-dev] [PATCH v2 1/2] lib/librte_pmd_i40e: set vlan filter fix
>>
>>
>>
>>> -----Original Message-----
>>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Huawei Xie
>>> Sent: Monday, November 10, 2014 10:46 AM
>>> To: dev@dpdk.org
>>> Subject: [dpdk-dev] [PATCH v2 1/2] lib/librte_pmd_i40e: set vlan filter fix
>>>
>>> ">> 5" rather than ">> 4"
>>>
>>> Signed-off-by: Huawei Xie <huawei.xie@intel.com>
>>> ---
>>>  lib/librte_pmd_i40e/i40e_ethdev.c | 7 ++-----
>>>  1 file changed, 2 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/lib/librte_pmd_i40e/i40e_ethdev.c
>>> b/lib/librte_pmd_i40e/i40e_ethdev.c
>>> index 5074262..c0cf3cf 100644
>>> --- a/lib/librte_pmd_i40e/i40e_ethdev.c
>>> +++ b/lib/librte_pmd_i40e/i40e_ethdev.c
>>> @@ -4048,14 +4048,11 @@ i40e_set_vlan_filter(struct i40e_vsi *vsi,  {
>>>  	uint32_t vid_idx, vid_bit;
>>>
>>> -#define UINT32_BIT_MASK      0x1F
>>> -#define VALID_VLAN_BIT_MASK  0xFFF
>>>  	/* VFTA is 32-bits size array, each element contains 32 vlan bits, Find the
>>>  	 *  element first, then find the bits it belongs to
>>>  	 */
>>> -	vid_idx = (uint32_t) ((vlan_id & VALID_VLAN_BIT_MASK) >>
>>> -		  sizeof(uint32_t));
>>> -	vid_bit = (uint32_t) (1 << (vlan_id & UINT32_BIT_MASK));
>>> +	vid_idx = (uint32_t) ((vlan_id >> 5 ) & 0x7F);

                                                                              
^^^^^^^^^^^^
                                                                       
       No need additional space after '5'
>>> +	vid_bit = (uint32_t) (1 << (vlan_id & 0x1F));
>> I don't understand why remove macros and use numeric instead?
> Those macros are wrongly defined. Correct macros are defined in second patch.

Is there any issue to swap your patch order, and use your defined macros
here? That would be much better if no other issue.

Thanks,
Michael
>>>  	if (on)
>>>  		vsi->vfta[vid_idx] |= vid_bit;
>>> --
>>> 1.8.1.4
>> Regards,
>> Helin


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

* Re: [dpdk-dev] [PATCH v2 1/2] lib/librte_pmd_i40e: set vlan filter fix
  2014-11-24  8:32       ` Qiu, Michael
@ 2014-11-24 21:41         ` Xie, Huawei
  0 siblings, 0 replies; 11+ messages in thread
From: Xie, Huawei @ 2014-11-24 21:41 UTC (permalink / raw)
  To: Qiu, Michael, Zhang, Helin, dev



> -----Original Message-----
> From: Qiu, Michael
> Sent: Monday, November 24, 2014 1:33 AM
> To: Xie, Huawei; Zhang, Helin; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v2 1/2] lib/librte_pmd_i40e: set vlan filter fix
> 
> On 11/10/2014 2:42 PM, Xie, Huawei wrote:
> >
> >> -----Original Message-----
> >> From: Zhang, Helin
> >> Sent: Sunday, November 09, 2014 10:09 PM
> >> To: Xie, Huawei; dev@dpdk.org
> >> Subject: RE: [dpdk-dev] [PATCH v2 1/2] lib/librte_pmd_i40e: set vlan filter fix
> >>
> >>
> >>
> >>> -----Original Message-----
> >>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Huawei Xie
> >>> Sent: Monday, November 10, 2014 10:46 AM
> >>> To: dev@dpdk.org
> >>> Subject: [dpdk-dev] [PATCH v2 1/2] lib/librte_pmd_i40e: set vlan filter fix
> >>>
> >>> ">> 5" rather than ">> 4"
> >>>
> >>> Signed-off-by: Huawei Xie <huawei.xie@intel.com>
> >>> ---
> >>>  lib/librte_pmd_i40e/i40e_ethdev.c | 7 ++-----
> >>>  1 file changed, 2 insertions(+), 5 deletions(-)
> >>>
> >>> diff --git a/lib/librte_pmd_i40e/i40e_ethdev.c
> >>> b/lib/librte_pmd_i40e/i40e_ethdev.c
> >>> index 5074262..c0cf3cf 100644
> >>> --- a/lib/librte_pmd_i40e/i40e_ethdev.c
> >>> +++ b/lib/librte_pmd_i40e/i40e_ethdev.c
> >>> @@ -4048,14 +4048,11 @@ i40e_set_vlan_filter(struct i40e_vsi *vsi,  {
> >>>  	uint32_t vid_idx, vid_bit;
> >>>
> >>> -#define UINT32_BIT_MASK      0x1F
> >>> -#define VALID_VLAN_BIT_MASK  0xFFF
> >>>  	/* VFTA is 32-bits size array, each element contains 32 vlan bits, Find the
> >>>  	 *  element first, then find the bits it belongs to
> >>>  	 */
> >>> -	vid_idx = (uint32_t) ((vlan_id & VALID_VLAN_BIT_MASK) >>
> >>> -		  sizeof(uint32_t));
> >>> -	vid_bit = (uint32_t) (1 << (vlan_id & UINT32_BIT_MASK));
> >>> +	vid_idx = (uint32_t) ((vlan_id >> 5 ) & 0x7F);
> 
> 
> ^^^^^^^^^^^^
> 
>        No need additional space after '5'
> >>> +	vid_bit = (uint32_t) (1 << (vlan_id & 0x1F));
> >> I don't understand why remove macros and use numeric instead?
> > Those macros are wrongly defined. Correct macros are defined in second
> patch.
> 
> Is there any issue to swap your patch order, and use your defined macros
> here? That would be much better if no other issue.

The first patch shows clearly what we fixes.
The second patch use MACROs for better code.
> 
> Thanks,
> Michael
> >>>  	if (on)
> >>>  		vsi->vfta[vid_idx] |= vid_bit;
> >>> --
> >>> 1.8.1.4
> >> Regards,
> >> Helin

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

end of thread, other threads:[~2014-11-24 21:31 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-11-10  2:46 [dpdk-dev] [PATCH v2 0/2] lib/librte_pmd_i40e: set vlan filter fix Huawei Xie
2014-11-10  2:46 ` [dpdk-dev] [PATCH v2 1/2] " Huawei Xie
2014-11-10  5:08   ` Zhang, Helin
2014-11-10  6:42     ` Xie, Huawei
2014-11-24  8:32       ` Qiu, Michael
2014-11-24 21:41         ` Xie, Huawei
2014-11-10  2:46 ` [dpdk-dev] [PATCH v2 2/2] lib/librte_pmd_i40e: add I40E_VFTA_IDX and I40E_VFTA_BIT macros for VFTA related operation Huawei Xie
2014-11-10  5:08   ` Zhang, Helin
2014-11-10  6:52     ` Xie, Huawei
2014-11-10  8:25 ` [dpdk-dev] [PATCH v2 0/2] lib/librte_pmd_i40e: set vlan filter fix Chen, Jing D
2014-11-21 20:47   ` Xie, Huawei

DPDK patches and discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ https://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git