* [dpdk-stable] [PATCH 1/2] net/ixgbe: fix defects of macro in VF [not found] <20200508015913.48764-1-guinanx.sun@intel.com> @ 2020-05-08 1:59 ` Guinan Sun 2020-05-08 1:59 ` [dpdk-stable] [PATCH 2/2] net/e1000: " Guinan Sun [not found] ` <20200508044618.70535-1-guinanx.sun@intel.com> 2 siblings, 0 replies; 10+ messages in thread From: Guinan Sun @ 2020-05-08 1:59 UTC (permalink / raw) To: dev; +Cc: Guinan Sun, stable The defects in the macros UPDATE_VF_STAT and UPDATE_VF_STAT_36BIT exist. If latest is less than last, we will get wrong result. The patch fixes the defect. Fixes: abf7275bbaa2 ("ixgbe: move to drivers/net/") Cc: stable@dpdk.org Signed-off-by: Guinan Sun <guinanx.sun@intel.com> --- drivers/net/ixgbe/ixgbe_ethdev.c | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c index cf5f1fe70..08a964399 100644 --- a/drivers/net/ixgbe/ixgbe_ethdev.c +++ b/drivers/net/ixgbe/ixgbe_ethdev.c @@ -94,6 +94,10 @@ #define IXGBE_4_BIT_MASK RTE_LEN2MASK(IXGBE_4_BIT_WIDTH, uint8_t) #define IXGBE_8_BIT_WIDTH CHAR_BIT #define IXGBE_8_BIT_MASK UINT8_MAX +#define IXGBE_32_BIT_WIDTH (CHAR_BIT * 4) +#define IXGBE_32_BIT_MASK RTE_LEN2MASK(IXGBE_32_BIT_WIDTH, uint32_t) +#define IXGBE_36_BIT_WIDTH (CHAR_BIT * 4 + IXGBE_4_BIT_WIDTH) +#define IXGBE_36_BIT_MASK RTE_LEN2MASK(IXGBE_36_BIT_WIDTH, uint64_t) #define IXGBEVF_PMD_NAME "rte_ixgbevf_pmd" /* PMD name */ @@ -388,7 +392,13 @@ static int ixgbe_wait_for_link_up(struct ixgbe_hw *hw); #define UPDATE_VF_STAT(reg, last, cur) \ { \ uint32_t latest = IXGBE_READ_REG(hw, reg); \ - cur += (latest - last) & UINT_MAX; \ + u64 stat = 0; \ + if (latest >= last) \ + stat = latest - last; \ + else \ + stat = (u64)((latest + \ + ((u64)1 << IXGBE_32_BIT_WIDTH)) - last);\ + cur += stat & IXGBE_32_BIT_MASK; \ last = latest; \ } @@ -397,7 +407,13 @@ static int ixgbe_wait_for_link_up(struct ixgbe_hw *hw); u64 new_lsb = IXGBE_READ_REG(hw, lsb); \ u64 new_msb = IXGBE_READ_REG(hw, msb); \ u64 latest = ((new_msb << 32) | new_lsb); \ - cur += (0x1000000000LL + latest - last) & 0xFFFFFFFFFLL; \ + u64 stat = 0; \ + if (latest >= last) \ + stat = latest - last; \ + else \ + stat = (u64)((latest + \ + ((u64)1 << IXGBE_36_BIT_WIDTH)) - last); \ + cur += stat & IXGBE_36_BIT_MASK; \ last = latest; \ } -- 2.17.1 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [dpdk-stable] [PATCH 2/2] net/e1000: fix defects of macro in VF [not found] <20200508015913.48764-1-guinanx.sun@intel.com> 2020-05-08 1:59 ` [dpdk-stable] [PATCH 1/2] net/ixgbe: fix defects of macro in VF Guinan Sun @ 2020-05-08 1:59 ` Guinan Sun [not found] ` <20200508044618.70535-1-guinanx.sun@intel.com> 2 siblings, 0 replies; 10+ messages in thread From: Guinan Sun @ 2020-05-08 1:59 UTC (permalink / raw) To: dev; +Cc: Guinan Sun, stable The defects in the macros UPDATE_VF_STAT and UPDATE_VF_STAT_36BIT exist. If latest is less than last, we will get wrong result. The patch fixes the defect. Fixes: d15fcf76c8b7 ("e1000: move to drivers/net/") Cc: stable@dpdk.org Signed-off-by: Guinan Sun <guinanx.sun@intel.com> --- drivers/net/e1000/igb_ethdev.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c index 520fba8fa..d6032fd65 100644 --- a/drivers/net/e1000/igb_ethdev.c +++ b/drivers/net/e1000/igb_ethdev.c @@ -47,6 +47,8 @@ #define IGB_4_BIT_MASK RTE_LEN2MASK(IGB_4_BIT_WIDTH, uint8_t) #define IGB_8_BIT_WIDTH CHAR_BIT #define IGB_8_BIT_MASK UINT8_MAX +#define IGB_32_BIT_WIDTH (CHAR_BIT * 4) +#define IGB_32_BIT_MASK RTE_LEN2MASK(IGB_32_BIT_WIDTH, uint32_t) /* Additional timesync values. */ #define E1000_CYCLECOUNTER_MASK 0xffffffffffffffffULL @@ -263,9 +265,15 @@ static int igb_filter_restore(struct rte_eth_dev *dev); */ #define UPDATE_VF_STAT(reg, last, cur) \ { \ - u32 latest = E1000_READ_REG(hw, reg); \ - cur += (latest - last) & UINT_MAX; \ - last = latest; \ + uint64_t latest = E1000_READ_REG(hw, reg); \ + uint64_t stat = 0; \ + if (latest >= last) \ + stat = latest - last; \ + else \ + stat = (uint64_t)((latest + \ + ((uint64_t)1 << IGB_32_BIT_WIDTH)) - last);\ + cur += stat & IGB_32_BIT_MASK; \ + last = latest; \ } #define IGB_FC_PAUSE_TIME 0x0680 -- 2.17.1 ^ permalink raw reply [flat|nested] 10+ messages in thread
[parent not found: <20200508044618.70535-1-guinanx.sun@intel.com>]
* [dpdk-stable] [PATCH v2 1/2] net/ixgbe: fix defects of macro in VF [not found] ` <20200508044618.70535-1-guinanx.sun@intel.com> @ 2020-05-08 4:46 ` Guinan Sun 2020-05-08 4:46 ` [dpdk-stable] [PATCH v2 2/2] net/e1000: " Guinan Sun 1 sibling, 0 replies; 10+ messages in thread From: Guinan Sun @ 2020-05-08 4:46 UTC (permalink / raw) To: dev; +Cc: Guinan Sun, stable The defects in the macros UPDATE_VF_STAT and UPDATE_VF_STAT_36BIT exist. If latest is less than last, we will get wrong result. The patch fixes the defect. Fixes: abf7275bbaa2 ("ixgbe: move to drivers/net/") Cc: stable@dpdk.org Signed-off-by: Guinan Sun <guinanx.sun@intel.com> --- v2 changes: * Aligned line-continuation character "\". --- drivers/net/ixgbe/ixgbe_ethdev.c | 32 ++++++++++++++++++++++++-------- 1 file changed, 24 insertions(+), 8 deletions(-) diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c index cf5f1fe70..57368c79c 100644 --- a/drivers/net/ixgbe/ixgbe_ethdev.c +++ b/drivers/net/ixgbe/ixgbe_ethdev.c @@ -94,6 +94,10 @@ #define IXGBE_4_BIT_MASK RTE_LEN2MASK(IXGBE_4_BIT_WIDTH, uint8_t) #define IXGBE_8_BIT_WIDTH CHAR_BIT #define IXGBE_8_BIT_MASK UINT8_MAX +#define IXGBE_32_BIT_WIDTH (CHAR_BIT * 4) +#define IXGBE_32_BIT_MASK RTE_LEN2MASK(IXGBE_32_BIT_WIDTH, uint32_t) +#define IXGBE_36_BIT_WIDTH (CHAR_BIT * 4 + IXGBE_4_BIT_WIDTH) +#define IXGBE_36_BIT_MASK RTE_LEN2MASK(IXGBE_36_BIT_WIDTH, uint64_t) #define IXGBEVF_PMD_NAME "rte_ixgbevf_pmd" /* PMD name */ @@ -388,17 +392,29 @@ static int ixgbe_wait_for_link_up(struct ixgbe_hw *hw); #define UPDATE_VF_STAT(reg, last, cur) \ { \ uint32_t latest = IXGBE_READ_REG(hw, reg); \ - cur += (latest - last) & UINT_MAX; \ + u64 stat = 0; \ + if (latest >= last) \ + stat = latest - last; \ + else \ + stat = (u64)((latest + \ + ((u64)1 << IXGBE_32_BIT_WIDTH)) - last);\ + cur += stat & IXGBE_32_BIT_MASK; \ last = latest; \ } -#define UPDATE_VF_STAT_36BIT(lsb, msb, last, cur) \ -{ \ - u64 new_lsb = IXGBE_READ_REG(hw, lsb); \ - u64 new_msb = IXGBE_READ_REG(hw, msb); \ - u64 latest = ((new_msb << 32) | new_lsb); \ - cur += (0x1000000000LL + latest - last) & 0xFFFFFFFFFLL; \ - last = latest; \ +#define UPDATE_VF_STAT_36BIT(lsb, msb, last, cur) \ +{ \ + u64 new_lsb = IXGBE_READ_REG(hw, lsb); \ + u64 new_msb = IXGBE_READ_REG(hw, msb); \ + u64 latest = ((new_msb << 32) | new_lsb); \ + u64 stat = 0; \ + if (latest >= last) \ + stat = latest - last; \ + else \ + stat = (u64)((latest + \ + ((u64)1 << IXGBE_36_BIT_WIDTH)) - last);\ + cur += stat & IXGBE_36_BIT_MASK; \ + last = latest; \ } #define IXGBE_SET_HWSTRIP(h, q) do {\ -- 2.17.1 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [dpdk-stable] [PATCH v2 2/2] net/e1000: fix defects of macro in VF [not found] ` <20200508044618.70535-1-guinanx.sun@intel.com> 2020-05-08 4:46 ` [dpdk-stable] [PATCH v2 1/2] net/ixgbe: " Guinan Sun @ 2020-05-08 4:46 ` Guinan Sun 2020-05-18 1:24 ` Ye Xiaolong 2020-05-18 7:21 ` Zhao1, Wei 1 sibling, 2 replies; 10+ messages in thread From: Guinan Sun @ 2020-05-08 4:46 UTC (permalink / raw) To: dev; +Cc: Guinan Sun, stable The defects in the macros UPDATE_VF_STAT and UPDATE_VF_STAT_36BIT exist. If latest is less than last, we will get wrong result. The patch fixes the defect. Fixes: d15fcf76c8b7 ("e1000: move to drivers/net/") Cc: stable@dpdk.org Signed-off-by: Guinan Sun <guinanx.sun@intel.com> --- v2 changes: * Aligned line-continuation character "\". --- drivers/net/e1000/igb_ethdev.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c index 520fba8fa..4cd4e55c0 100644 --- a/drivers/net/e1000/igb_ethdev.c +++ b/drivers/net/e1000/igb_ethdev.c @@ -47,6 +47,8 @@ #define IGB_4_BIT_MASK RTE_LEN2MASK(IGB_4_BIT_WIDTH, uint8_t) #define IGB_8_BIT_WIDTH CHAR_BIT #define IGB_8_BIT_MASK UINT8_MAX +#define IGB_32_BIT_WIDTH (CHAR_BIT * 4) +#define IGB_32_BIT_MASK RTE_LEN2MASK(IGB_32_BIT_WIDTH, uint32_t) /* Additional timesync values. */ #define E1000_CYCLECOUNTER_MASK 0xffffffffffffffffULL @@ -261,11 +263,17 @@ static int igb_filter_restore(struct rte_eth_dev *dev); /* * Define VF Stats MACRO for Non "cleared on read" register */ -#define UPDATE_VF_STAT(reg, last, cur) \ -{ \ - u32 latest = E1000_READ_REG(hw, reg); \ - cur += (latest - last) & UINT_MAX; \ - last = latest; \ +#define UPDATE_VF_STAT(reg, last, cur) \ +{ \ + uint64_t latest = E1000_READ_REG(hw, reg); \ + uint64_t stat = 0; \ + if (latest >= last) \ + stat = latest - last; \ + else \ + stat = (uint64_t)((latest + \ + ((uint64_t)1 << IGB_32_BIT_WIDTH)) - last);\ + cur += stat & IGB_32_BIT_MASK; \ + last = latest; \ } #define IGB_FC_PAUSE_TIME 0x0680 -- 2.17.1 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [dpdk-stable] [PATCH v2 2/2] net/e1000: fix defects of macro in VF 2020-05-08 4:46 ` [dpdk-stable] [PATCH v2 2/2] net/e1000: " Guinan Sun @ 2020-05-18 1:24 ` Ye Xiaolong 2020-05-18 6:48 ` [dpdk-stable] [dpdk-dev] " Zhao1, Wei 2020-05-18 7:21 ` Zhao1, Wei 1 sibling, 1 reply; 10+ messages in thread From: Ye Xiaolong @ 2020-05-18 1:24 UTC (permalink / raw) To: Guinan Sun; +Cc: dev, stable Hi, guinan On 05/08, Guinan Sun wrote: >The defects in the macros UPDATE_VF_STAT and UPDATE_VF_STAT_36BIT exist. >If latest is less than last, we will get wrong result. >The patch fixes the defect. There was similar patch before, https://patches.dpdk.org/patch/65131/, if I understand it correctly, you are trying to solve the rollover issue, right? Could you find the Ferruh's comment and check if this is a real issue? > >Fixes: d15fcf76c8b7 ("e1000: move to drivers/net/") This fix commit isn't correct. Thanks, Xiaolong >Cc: stable@dpdk.org > >Signed-off-by: Guinan Sun <guinanx.sun@intel.com> >--- >v2 changes: >* Aligned line-continuation character "\". >--- > drivers/net/e1000/igb_ethdev.c | 18 +++++++++++++----- > 1 file changed, 13 insertions(+), 5 deletions(-) > >diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c >index 520fba8fa..4cd4e55c0 100644 >--- a/drivers/net/e1000/igb_ethdev.c >+++ b/drivers/net/e1000/igb_ethdev.c >@@ -47,6 +47,8 @@ > #define IGB_4_BIT_MASK RTE_LEN2MASK(IGB_4_BIT_WIDTH, uint8_t) > #define IGB_8_BIT_WIDTH CHAR_BIT > #define IGB_8_BIT_MASK UINT8_MAX >+#define IGB_32_BIT_WIDTH (CHAR_BIT * 4) >+#define IGB_32_BIT_MASK RTE_LEN2MASK(IGB_32_BIT_WIDTH, uint32_t) > > /* Additional timesync values. */ > #define E1000_CYCLECOUNTER_MASK 0xffffffffffffffffULL >@@ -261,11 +263,17 @@ static int igb_filter_restore(struct rte_eth_dev *dev); > /* > * Define VF Stats MACRO for Non "cleared on read" register > */ >-#define UPDATE_VF_STAT(reg, last, cur) \ >-{ \ >- u32 latest = E1000_READ_REG(hw, reg); \ >- cur += (latest - last) & UINT_MAX; \ >- last = latest; \ >+#define UPDATE_VF_STAT(reg, last, cur) \ >+{ \ >+ uint64_t latest = E1000_READ_REG(hw, reg); \ >+ uint64_t stat = 0; \ >+ if (latest >= last) \ >+ stat = latest - last; \ >+ else \ >+ stat = (uint64_t)((latest + \ >+ ((uint64_t)1 << IGB_32_BIT_WIDTH)) - last);\ >+ cur += stat & IGB_32_BIT_MASK; \ >+ last = latest; \ > } > > #define IGB_FC_PAUSE_TIME 0x0680 >-- >2.17.1 > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [dpdk-stable] [dpdk-dev] [PATCH v2 2/2] net/e1000: fix defects of macro in VF 2020-05-18 1:24 ` Ye Xiaolong @ 2020-05-18 6:48 ` Zhao1, Wei 2020-05-18 23:39 ` Ye Xiaolong 0 siblings, 1 reply; 10+ messages in thread From: Zhao1, Wei @ 2020-05-18 6:48 UTC (permalink / raw) To: Ye, Xiaolong, Sun, GuinanX; +Cc: dev, stable, Min, JiaqiX, Yigit, Ferruh Hi, xiaolong > -----Original Message----- > From: dev <dev-bounces@dpdk.org> On Behalf Of Ye Xiaolong > Sent: Monday, May 18, 2020 9:25 AM > To: Sun, GuinanX <guinanx.sun@intel.com> > Cc: dev@dpdk.org; stable@dpdk.org > Subject: Re: [dpdk-dev] [dpdk-stable] [PATCH v2 2/2] net/e1000: fix defects of > macro in VF > > Hi, guinan > > On 05/08, Guinan Sun wrote: > >The defects in the macros UPDATE_VF_STAT and UPDATE_VF_STAT_36BIT > exist. > >If latest is less than last, we will get wrong result. > >The patch fixes the defect. > > There was similar patch before, https://patches.dpdk.org/patch/65131/, if I > understand it correctly, you are trying to solve the rollover issue, right? > Could you find the Ferruh's comment and check if this is a real issue? this issue has not been fixed by now, we need this patch to fix it. If (latest < last), there will be issue, is that right? > > > > >Fixes: d15fcf76c8b7 ("e1000: move to drivers/net/") > > This fix commit isn't correct. > > Thanks, > Xiaolong > > >Cc: stable@dpdk.org > > > > > > >Signed-off-by: Guinan Sun <guinanx.sun@intel.com> > >--- > >v2 changes: > >* Aligned line-continuation character "\". > >--- > > drivers/net/e1000/igb_ethdev.c | 18 +++++++++++++----- > > 1 file changed, 13 insertions(+), 5 deletions(-) > > > >diff --git a/drivers/net/e1000/igb_ethdev.c > >b/drivers/net/e1000/igb_ethdev.c index 520fba8fa..4cd4e55c0 100644 > >--- a/drivers/net/e1000/igb_ethdev.c > >+++ b/drivers/net/e1000/igb_ethdev.c > >@@ -47,6 +47,8 @@ > > #define IGB_4_BIT_MASK RTE_LEN2MASK(IGB_4_BIT_WIDTH, uint8_t) > > #define IGB_8_BIT_WIDTH CHAR_BIT > > #define IGB_8_BIT_MASK UINT8_MAX > >+#define IGB_32_BIT_WIDTH (CHAR_BIT * 4) #define IGB_32_BIT_MASK > >+RTE_LEN2MASK(IGB_32_BIT_WIDTH, uint32_t) > > > > /* Additional timesync values. */ > > #define E1000_CYCLECOUNTER_MASK 0xffffffffffffffffULL > >@@ -261,11 +263,17 @@ static int igb_filter_restore(struct rte_eth_dev > >*dev); > > /* > > * Define VF Stats MACRO for Non "cleared on read" register > > */ > >-#define UPDATE_VF_STAT(reg, last, cur) \ > >-{ \ > >- u32 latest = E1000_READ_REG(hw, reg); \ > >- cur += (latest - last) & UINT_MAX; \ > >- last = latest; \ > >+#define UPDATE_VF_STAT(reg, last, cur) > \ > >+{ > \ > >+ uint64_t latest = E1000_READ_REG(hw, reg); \ > >+ uint64_t stat = 0; \ > >+ if (latest >= last) \ > >+ stat = latest - last; \ > >+ else \ > >+ stat = (uint64_t)((latest + \ > >+ ((uint64_t)1 << IGB_32_BIT_WIDTH)) - last);\ > >+ cur += stat & IGB_32_BIT_MASK; \ > >+ last = latest; \ > > } > > > > #define IGB_FC_PAUSE_TIME 0x0680 > >-- > >2.17.1 > > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [dpdk-stable] [dpdk-dev] [PATCH v2 2/2] net/e1000: fix defects of macro in VF 2020-05-18 6:48 ` [dpdk-stable] [dpdk-dev] " Zhao1, Wei @ 2020-05-18 23:39 ` Ye Xiaolong 2020-05-19 1:01 ` Zhao1, Wei 0 siblings, 1 reply; 10+ messages in thread From: Ye Xiaolong @ 2020-05-18 23:39 UTC (permalink / raw) To: Zhao1, Wei; +Cc: Sun, GuinanX, dev, stable, Min, JiaqiX, Yigit, Ferruh Hi, wei On 05/18, Zhao1, Wei wrote: >Hi, xiaolong > >> -----Original Message----- >> From: dev <dev-bounces@dpdk.org> On Behalf Of Ye Xiaolong >> Sent: Monday, May 18, 2020 9:25 AM >> To: Sun, GuinanX <guinanx.sun@intel.com> >> Cc: dev@dpdk.org; stable@dpdk.org >> Subject: Re: [dpdk-dev] [dpdk-stable] [PATCH v2 2/2] net/e1000: fix defects of >> macro in VF >> >> Hi, guinan >> >> On 05/08, Guinan Sun wrote: >> >The defects in the macros UPDATE_VF_STAT and UPDATE_VF_STAT_36BIT >> exist. >> >If latest is less than last, we will get wrong result. >> >The patch fixes the defect. >> >> There was similar patch before, https://patches.dpdk.org/patch/65131/, if I >> understand it correctly, you are trying to solve the rollover issue, right? >> Could you find the Ferruh's comment and check if this is a real issue? > >this issue has not been fixed by now, we need this patch to fix it. >If (latest < last), there will be issue, is that right? Actually the rollover case has been covered by original code. Thanks, Xiaolong > > >> >> > >> >Fixes: d15fcf76c8b7 ("e1000: move to drivers/net/") >> >> This fix commit isn't correct. >> >> Thanks, >> Xiaolong >> >> >Cc: stable@dpdk.org >> >> >> >> > >> >Signed-off-by: Guinan Sun <guinanx.sun@intel.com> >> >--- >> >v2 changes: >> >* Aligned line-continuation character "\". >> >--- >> > drivers/net/e1000/igb_ethdev.c | 18 +++++++++++++----- >> > 1 file changed, 13 insertions(+), 5 deletions(-) >> > >> >diff --git a/drivers/net/e1000/igb_ethdev.c >> >b/drivers/net/e1000/igb_ethdev.c index 520fba8fa..4cd4e55c0 100644 >> >--- a/drivers/net/e1000/igb_ethdev.c >> >+++ b/drivers/net/e1000/igb_ethdev.c >> >@@ -47,6 +47,8 @@ >> > #define IGB_4_BIT_MASK RTE_LEN2MASK(IGB_4_BIT_WIDTH, uint8_t) >> > #define IGB_8_BIT_WIDTH CHAR_BIT >> > #define IGB_8_BIT_MASK UINT8_MAX >> >+#define IGB_32_BIT_WIDTH (CHAR_BIT * 4) #define IGB_32_BIT_MASK >> >+RTE_LEN2MASK(IGB_32_BIT_WIDTH, uint32_t) >> > >> > /* Additional timesync values. */ >> > #define E1000_CYCLECOUNTER_MASK 0xffffffffffffffffULL >> >@@ -261,11 +263,17 @@ static int igb_filter_restore(struct rte_eth_dev >> >*dev); >> > /* >> > * Define VF Stats MACRO for Non "cleared on read" register >> > */ >> >-#define UPDATE_VF_STAT(reg, last, cur) \ >> >-{ \ >> >-u32 latest = E1000_READ_REG(hw, reg); \ >> >-cur += (latest - last) & UINT_MAX; \ >> >-last = latest; \ >> >+#define UPDATE_VF_STAT(reg, last, cur) >> \ >> >+{ >> \ >> >+uint64_t latest = E1000_READ_REG(hw, reg); \ >> >+uint64_t stat = 0; \ >> >+if (latest >= last) \ >> >+stat = latest - last; \ >> >+else \ >> >+stat = (uint64_t)((latest + \ >> >+((uint64_t)1 << IGB_32_BIT_WIDTH)) - last);\ >> >+cur += stat & IGB_32_BIT_MASK; \ >> >+last = latest; \ >> > } >> > >> > #define IGB_FC_PAUSE_TIME 0x0680 >> >-- >> >2.17.1 >> > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [dpdk-stable] [dpdk-dev] [PATCH v2 2/2] net/e1000: fix defects of macro in VF 2020-05-18 23:39 ` Ye Xiaolong @ 2020-05-19 1:01 ` Zhao1, Wei 0 siblings, 0 replies; 10+ messages in thread From: Zhao1, Wei @ 2020-05-19 1:01 UTC (permalink / raw) To: Ye, Xiaolong; +Cc: Sun, GuinanX, dev, stable, Min, JiaqiX, Yigit, Ferruh > -----Original Message----- > From: Ye, Xiaolong <xiaolong.ye@intel.com> > Sent: Tuesday, May 19, 2020 7:39 AM > To: Zhao1, Wei <wei.zhao1@intel.com> > Cc: Sun, GuinanX <guinanx.sun@intel.com>; dev@dpdk.org; stable@dpdk.org; > Min, JiaqiX <jiaqix.min@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com> > Subject: Re: [dpdk-dev] [dpdk-stable] [PATCH v2 2/2] net/e1000: fix defects of > macro in VF > > Hi, wei > > On 05/18, Zhao1, Wei wrote: > >Hi, xiaolong > > > >> -----Original Message----- > >> From: dev <dev-bounces@dpdk.org> On Behalf Of Ye Xiaolong > >> Sent: Monday, May 18, 2020 9:25 AM > >> To: Sun, GuinanX <guinanx.sun@intel.com> > >> Cc: dev@dpdk.org; stable@dpdk.org > >> Subject: Re: [dpdk-dev] [dpdk-stable] [PATCH v2 2/2] net/e1000: fix > >> defects of macro in VF > >> > >> Hi, guinan > >> > >> On 05/08, Guinan Sun wrote: > >> >The defects in the macros UPDATE_VF_STAT and UPDATE_VF_STAT_36BIT > >> exist. > >> >If latest is less than last, we will get wrong result. > >> >The patch fixes the defect. > >> > >> There was similar patch before, > >> https://patches.dpdk.org/patch/65131/, if I understand it correctly, you are > trying to solve the rollover issue, right? > >> Could you find the Ferruh's comment and check if this is a real issue? > > > >this issue has not been fixed by now, we need this patch to fix it. > >If (latest < last), there will be issue, is that right? > > Actually the rollover case has been covered by original code. Yes, you are right. > > Thanks, > Xiaolong > > > > > > > >> > >> > > >> >Fixes: d15fcf76c8b7 ("e1000: move to drivers/net/") > >> > >> This fix commit isn't correct. > >> > >> Thanks, > >> Xiaolong > >> > >> >Cc: stable@dpdk.org > >> > >> > >> > >> > > >> >Signed-off-by: Guinan Sun <guinanx.sun@intel.com> > >> >--- > >> >v2 changes: > >> >* Aligned line-continuation character "\". > >> >--- > >> > drivers/net/e1000/igb_ethdev.c | 18 +++++++++++++----- > >> > 1 file changed, 13 insertions(+), 5 deletions(-) > >> > > >> >diff --git a/drivers/net/e1000/igb_ethdev.c > >> >b/drivers/net/e1000/igb_ethdev.c index 520fba8fa..4cd4e55c0 100644 > >> >--- a/drivers/net/e1000/igb_ethdev.c > >> >+++ b/drivers/net/e1000/igb_ethdev.c > >> >@@ -47,6 +47,8 @@ > >> > #define IGB_4_BIT_MASK RTE_LEN2MASK(IGB_4_BIT_WIDTH, > uint8_t) > >> > #define IGB_8_BIT_WIDTH CHAR_BIT > >> > #define IGB_8_BIT_MASK UINT8_MAX > >> >+#define IGB_32_BIT_WIDTH (CHAR_BIT * 4) #define IGB_32_BIT_MASK > >> >+RTE_LEN2MASK(IGB_32_BIT_WIDTH, uint32_t) > >> > > >> > /* Additional timesync values. */ > >> > #define E1000_CYCLECOUNTER_MASK 0xffffffffffffffffULL > >> >@@ -261,11 +263,17 @@ static int igb_filter_restore(struct > >> >rte_eth_dev *dev); > >> > /* > >> > * Define VF Stats MACRO for Non "cleared on read" register > >> > */ > >> >-#define UPDATE_VF_STAT(reg, last, cur) \ > >> >-{ \ > >> >-u32 latest = E1000_READ_REG(hw, reg); \ > >> >-cur += (latest - last) & UINT_MAX; \ > >> >-last = latest; \ > >> >+#define UPDATE_VF_STAT(reg, last, cur) > >> \ > >> >+{ > >> \ > >> >+uint64_t latest = E1000_READ_REG(hw, reg); \ > >> >+uint64_t stat = 0; \ > >> >+if (latest >= last) \ > >> >+stat = latest - last; \ > >> >+else \ > >> >+stat = (uint64_t)((latest + \ > >> >+((uint64_t)1 << IGB_32_BIT_WIDTH)) - last);\ > >> >+cur += stat & IGB_32_BIT_MASK; \ > >> >+last = latest; \ > >> > } > >> > > >> > #define IGB_FC_PAUSE_TIME 0x0680 > >> >-- > >> >2.17.1 > >> > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [dpdk-stable] [dpdk-dev] [PATCH v2 2/2] net/e1000: fix defects of macro in VF 2020-05-08 4:46 ` [dpdk-stable] [PATCH v2 2/2] net/e1000: " Guinan Sun 2020-05-18 1:24 ` Ye Xiaolong @ 2020-05-18 7:21 ` Zhao1, Wei 1 sibling, 0 replies; 10+ messages in thread From: Zhao1, Wei @ 2020-05-18 7:21 UTC (permalink / raw) To: Sun, GuinanX, dev; +Cc: Sun, GuinanX, stable, Min, JiaqiX Hi, jiaqi > -----Original Message----- > From: dev <dev-bounces@dpdk.org> On Behalf Of Guinan Sun > Sent: Friday, May 8, 2020 12:46 PM > To: dev@dpdk.org > Cc: Sun, GuinanX <guinanx.sun@intel.com>; stable@dpdk.org > Subject: [dpdk-dev] [PATCH v2 2/2] net/e1000: fix defects of macro in VF > > The defects in the macros UPDATE_VF_STAT and UPDATE_VF_STAT_36BIT > exist. > If latest is less than last, we will get wrong result. > The patch fixes the defect. > > Fixes: d15fcf76c8b7 ("e1000: move to drivers/net/") > Cc: stable@dpdk.org > > Signed-off-by: Guinan Sun <guinanx.sun@intel.com> > --- > v2 changes: > * Aligned line-continuation character "\". > --- > drivers/net/e1000/igb_ethdev.c | 18 +++++++++++++----- > 1 file changed, 13 insertions(+), 5 deletions(-) > > diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c > index 520fba8fa..4cd4e55c0 100644 > --- a/drivers/net/e1000/igb_ethdev.c > +++ b/drivers/net/e1000/igb_ethdev.c > @@ -47,6 +47,8 @@ > #define IGB_4_BIT_MASK RTE_LEN2MASK(IGB_4_BIT_WIDTH, uint8_t) > #define IGB_8_BIT_WIDTH CHAR_BIT > #define IGB_8_BIT_MASK UINT8_MAX > +#define IGB_32_BIT_WIDTH (CHAR_BIT * 4) #define IGB_32_BIT_MASK > +RTE_LEN2MASK(IGB_32_BIT_WIDTH, uint32_t) > > /* Additional timesync values. */ > #define E1000_CYCLECOUNTER_MASK 0xffffffffffffffffULL > @@ -261,11 +263,17 @@ static int igb_filter_restore(struct rte_eth_dev > *dev); > /* > * Define VF Stats MACRO for Non "cleared on read" register > */ > -#define UPDATE_VF_STAT(reg, last, cur) \ > -{ \ > - u32 latest = E1000_READ_REG(hw, reg); \ > - cur += (latest - last) & UINT_MAX; \ > - last = latest; \ > +#define UPDATE_VF_STAT(reg, last, cur) \ > +{ > \ > + uint64_t latest = E1000_READ_REG(hw, reg); \ > + uint64_t stat = 0; \ Why do use 64 bit for register read？ Igb register only 32 bit width? > + if (latest >= last) \ > + stat = latest - last; \ > + else \ > + stat = (uint64_t)((latest + \ > + ((uint64_t)1 << IGB_32_BIT_WIDTH)) - last);\ > + cur += stat & IGB_32_BIT_MASK; \ > + last = latest; \ > } > > #define IGB_FC_PAUSE_TIME 0x0680 > -- > 2.17.1 ^ permalink raw reply [flat|nested] 10+ messages in thread
[parent not found: <20200507061642.64969-1-guinanx.sun@intel.com>]
* [dpdk-stable] [PATCH 1/2] net/ixgbe: fix defects of macro in VF [not found] <20200507061642.64969-1-guinanx.sun@intel.com> @ 2020-05-07 6:16 ` Guinan Sun 0 siblings, 0 replies; 10+ messages in thread From: Guinan Sun @ 2020-05-07 6:16 UTC (permalink / raw) To: dev; +Cc: Guinan Sun, stable The defects in the macros UPDATE_VF_STAT and UPDATE_VF_STAT_36BIT exist. If latest is less than last, we will get wrong result. The patch fixes the defect. Fixes: abf7275bbaa2 ("ixgbe: move to drivers/net/") Cc: stable@dpdk.org Signed-off-by: Guinan Sun <guinanx.sun@intel.com> --- drivers/net/ixgbe/ixgbe_ethdev.c | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c index cf5f1fe70..08a964399 100644 --- a/drivers/net/ixgbe/ixgbe_ethdev.c +++ b/drivers/net/ixgbe/ixgbe_ethdev.c @@ -94,6 +94,10 @@ #define IXGBE_4_BIT_MASK RTE_LEN2MASK(IXGBE_4_BIT_WIDTH, uint8_t) #define IXGBE_8_BIT_WIDTH CHAR_BIT #define IXGBE_8_BIT_MASK UINT8_MAX +#define IXGBE_32_BIT_WIDTH (CHAR_BIT * 4) +#define IXGBE_32_BIT_MASK RTE_LEN2MASK(IXGBE_32_BIT_WIDTH, uint32_t) +#define IXGBE_36_BIT_WIDTH (CHAR_BIT * 4 + IXGBE_4_BIT_WIDTH) +#define IXGBE_36_BIT_MASK RTE_LEN2MASK(IXGBE_36_BIT_WIDTH, uint64_t) #define IXGBEVF_PMD_NAME "rte_ixgbevf_pmd" /* PMD name */ @@ -388,7 +392,13 @@ static int ixgbe_wait_for_link_up(struct ixgbe_hw *hw); #define UPDATE_VF_STAT(reg, last, cur) \ { \ uint32_t latest = IXGBE_READ_REG(hw, reg); \ - cur += (latest - last) & UINT_MAX; \ + u64 stat = 0; \ + if (latest >= last) \ + stat = latest - last; \ + else \ + stat = (u64)((latest + \ + ((u64)1 << IXGBE_32_BIT_WIDTH)) - last);\ + cur += stat & IXGBE_32_BIT_MASK; \ last = latest; \ } @@ -397,7 +407,13 @@ static int ixgbe_wait_for_link_up(struct ixgbe_hw *hw); u64 new_lsb = IXGBE_READ_REG(hw, lsb); \ u64 new_msb = IXGBE_READ_REG(hw, msb); \ u64 latest = ((new_msb << 32) | new_lsb); \ - cur += (0x1000000000LL + latest - last) & 0xFFFFFFFFFLL; \ + u64 stat = 0; \ + if (latest >= last) \ + stat = latest - last; \ + else \ + stat = (u64)((latest + \ + ((u64)1 << IXGBE_36_BIT_WIDTH)) - last); \ + cur += stat & IXGBE_36_BIT_MASK; \ last = latest; \ } -- 2.17.1 ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2020-05-19 1:01 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20200508015913.48764-1-guinanx.sun@intel.com>
2020-05-08 1:59 ` [dpdk-stable] [PATCH 1/2] net/ixgbe: fix defects of macro in VF Guinan Sun
2020-05-08 1:59 ` [dpdk-stable] [PATCH 2/2] net/e1000: " Guinan Sun
[not found] ` <20200508044618.70535-1-guinanx.sun@intel.com>
2020-05-08 4:46 ` [dpdk-stable] [PATCH v2 1/2] net/ixgbe: " Guinan Sun
2020-05-08 4:46 ` [dpdk-stable] [PATCH v2 2/2] net/e1000: " Guinan Sun
2020-05-18 1:24 ` Ye Xiaolong
2020-05-18 6:48 ` [dpdk-stable] [dpdk-dev] " Zhao1, Wei
2020-05-18 23:39 ` Ye Xiaolong
2020-05-19 1:01 ` Zhao1, Wei
2020-05-18 7:21 ` Zhao1, Wei
[not found] <20200507061642.64969-1-guinanx.sun@intel.com>
2020-05-07 6:16 ` [dpdk-stable] [PATCH 1/2] net/ixgbe: " Guinan Sun
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).