diff mbox

[dpdk-dev,v2,1/2] lib/librte_pmd_i40e: set vlan filter fix

Message ID 1415587563-11512-2-git-send-email-huawei.xie@intel.com (mailing list archive)
State Changes Requested, archived
Headers show

Commit Message

Huawei Xie Nov. 10, 2014, 2:46 a.m. UTC
">> 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(-)

Comments

Helin Zhang Nov. 10, 2014, 5:08 a.m. UTC | #1
> -----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
Huawei Xie Nov. 10, 2014, 6:42 a.m. UTC | #2
> -----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
Michael Qiu Nov. 24, 2014, 8:32 a.m. UTC | #3
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
Huawei Xie Nov. 24, 2014, 9:41 p.m. UTC | #4
> -----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
diff mbox

Patch

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;