Hi Ferruh, > -----Original Message----- > From: Yigit, Ferruh > Sent: Tuesday, September 20, 2016 1:01 AM > To: Wang, Xiao W <xiao.w.wang at intel.com>; Lu, Wenzhuo > <wenzhuo.lu at intel.com> > Cc: dev at dpdk.org > Subject: Re: [dpdk-dev] [PATCH 01/39] net/ixgbe/base: fix delta check for > setting VFTA > > On 8/27/2016 4:47 PM, Xiao Wang wrote: > > The delta value rather than vfta_delta pointer should be checked. > > > > Fixes: b978f7b38c14 ("net/ixgbe/base: simplify VLAN management") > > > > Signed-off-by: Xiao Wang <xiao.w.wang at intel.com> > > --- > > drivers/net/ixgbe/base/ixgbe_82598.c | 6 +++--- > > drivers/net/ixgbe/base/ixgbe_api.c | 7 ++++--- > > drivers/net/ixgbe/base/ixgbe_common.c | 2 +- > > 3 files changed, 8 insertions(+), 7 deletions(-) > > > > diff --git a/drivers/net/ixgbe/base/ixgbe_82598.c > b/drivers/net/ixgbe/base/ixgbe_82598.c > > index db80880..724dcbb 100644 > > --- a/drivers/net/ixgbe/base/ixgbe_82598.c > > +++ b/drivers/net/ixgbe/base/ixgbe_82598.c > > @@ -995,19 +995,19 @@ STATIC s32 ixgbe_clear_vmdq_82598(struct > ixgbe_hw *hw, u32 rar, u32 vmdq) > > * @vlan: VLAN id to write to VLAN filter > > * @vind: VMDq output index that maps queue to VLAN id in VFTA > > * @vlan_on: boolean flag to turn on/off VLAN in VFTA > > - * @bypass_vlvf: boolean flag - unused > > + * @vlvf_bypass: boolean flag - unused > > bypass_vlvf -> vlvf_bypass belongs to different patch > > > * > > * Turn on/off specified VLAN in the VLAN filter table. > > **/ > > s32 ixgbe_set_vfta_82598(struct ixgbe_hw *hw, u32 vlan, u32 vind, > > - bool vlan_on, bool bypass_vlvf) > > + bool vlan_on, bool vlvf_bypass) > > { > > u32 regindex; > > u32 bitindex; > > u32 bits; > > u32 vftabyte; > > > > - UNREFERENCED_1PARAMETER(bypass_vlvf); > > + UNREFERENCED_1PARAMETER(vlvf_bypass); > > > > DEBUGFUNC("ixgbe_set_vfta_82598"); > > > > diff --git a/drivers/net/ixgbe/base/ixgbe_api.c > b/drivers/net/ixgbe/base/ixgbe_api.c > > index 1786867..5b721af 100644 > > --- a/drivers/net/ixgbe/base/ixgbe_api.c > > +++ b/drivers/net/ixgbe/base/ixgbe_api.c > > @@ -1090,7 +1090,7 @@ s32 ixgbe_set_vfta(struct ixgbe_hw *hw, u32 vlan, > u32 vind, bool vlan_on, > > bool vlvf_bypass) > > { > > return ixgbe_call_func(hw, hw->mac.ops.set_vfta, (hw, vlan, vind, > > - vlan_on, vlvf_bypass), > IXGBE_NOT_IMPLEMENTED); > > + vlan_on, vlvf_bypass), > IXGBE_NOT_IMPLEMENTED); > > } > > > > /** > > @@ -1100,7 +1100,7 @@ s32 ixgbe_set_vfta(struct ixgbe_hw *hw, u32 vlan, > u32 vind, bool vlan_on, > > * @vind: VMDq output index that maps queue to VLAN id in VLVFB > > * @vlan_on: boolean flag to turn on/off VLAN in VLVF > > * @vfta_delta: pointer to the difference between the current value of > > VFTA > > - * and the desired value > > + * and the desired value > > * @vfta: the desired value of the VFTA > > * @vlvf_bypass: boolean flag indicating updating the default pool is okay > > * > > @@ -1110,7 +1110,7 @@ s32 ixgbe_set_vlvf(struct ixgbe_hw *hw, u32 vlan, > u32 vind, bool vlan_on, > > u32 *vfta_delta, u32 vfta, bool vlvf_bypass) > > { > > return ixgbe_call_func(hw, hw->mac.ops.set_vlvf, (hw, vlan, vind, > > - vlan_on, vfta_delta, vfta, vlvf_bypass), > > + vlan_on, vfta_delta, vfta, vlvf_bypass), > > IXGBE_NOT_IMPLEMENTED); > > } > > > > @@ -1659,6 +1659,7 @@ void ixgbe_init_swfw_semaphore(struct ixgbe_hw > *hw) > > hw->mac.ops.init_swfw_sync(hw); > > } > > > > + > > unrelated whitespace modifications > > > > void ixgbe_disable_rx(struct ixgbe_hw *hw) > > { > > if (hw->mac.ops.disable_rx) > > diff --git a/drivers/net/ixgbe/base/ixgbe_common.c > b/drivers/net/ixgbe/base/ixgbe_common.c > > index 811875a..161bf32 100644 > > --- a/drivers/net/ixgbe/base/ixgbe_common.c > > +++ b/drivers/net/ixgbe/base/ixgbe_common.c > > @@ -3967,7 +3967,7 @@ s32 ixgbe_set_vlvf_generic(struct ixgbe_hw *hw, > u32 vlan, u32 vind, > > * we run the risk of stray packets leaking into > > * the PF via the default pool > > */ > > - if (vfta_delta) > > + if (*vfta_delta) > > This seems only update mentioned in patch commit log. > > What about extracting all other clean up modifications into a new patch, > other patches also have similar fixes, all can go into that cleanup patch?
Good advice, I will put all the minor misc modifications into a separate patch. Thanks. > > > IXGBE_WRITE_REG(hw, IXGBE_VFTA(vlan / 32), vfta); > > > > /* disable VLVF and clear remaining bit from pool */ > > >