> -----Original Message----- > From: Qiu, Michael > Sent: Monday, November 24, 2014 1:33 AM > To: Xie, Huawei; Zhang, Helin; dev at 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 at 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 at dpdk.org] On Behalf Of Huawei Xie > >>> Sent: Monday, November 10, 2014 10:46 AM > >>> To: dev at 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 at 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
