* [dpdk-dev] [PATCH] net/szedata2: fix incorrect device memory access
@ 2017-01-24 10:49 Matej Vido
2017-01-24 11:58 ` Ferruh Yigit
2017-01-24 15:24 ` Ferruh Yigit
0 siblings, 2 replies; 10+ messages in thread
From: Matej Vido @ 2017-01-24 10:49 UTC (permalink / raw)
To: dev
Fixes: 8acba705b119 ("net/szedata2: localize handling of PCI resources")
Signed-off-by: Matej Vido <vido@cesnet.cz>
---
drivers/net/szedata2/rte_eth_szedata2.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/szedata2/rte_eth_szedata2.h b/drivers/net/szedata2/rte_eth_szedata2.h
index b58adb6..afe8a38 100644
--- a/drivers/net/szedata2/rte_eth_szedata2.h
+++ b/drivers/net/szedata2/rte_eth_szedata2.h
@@ -192,7 +192,7 @@ struct szedata {
}
#define SZEDATA2_PCI_RESOURCE_PTR(rsc, offset, type) \
- ((type)((uint8_t *)(rsc)->addr) + (offset))
+ ((type)(((uint8_t *)(rsc)->addr) + (offset)))
enum szedata2_link_speed {
SZEDATA2_LINK_SPEED_DEFAULT = 0,
--
1.8.4
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [dpdk-dev] [PATCH] net/szedata2: fix incorrect device memory access 2017-01-24 10:49 [dpdk-dev] [PATCH] net/szedata2: fix incorrect device memory access Matej Vido @ 2017-01-24 11:58 ` Ferruh Yigit 2017-01-24 14:02 ` Matej Vido 2017-01-24 15:24 ` Ferruh Yigit 1 sibling, 1 reply; 10+ messages in thread From: Ferruh Yigit @ 2017-01-24 11:58 UTC (permalink / raw) To: Matej Vido, dev On 1/24/2017 10:49 AM, Matej Vido wrote: > Fixes: 8acba705b119 ("net/szedata2: localize handling of PCI resources") > > Signed-off-by: Matej Vido <vido@cesnet.cz> Unrelated from this patch, in maintainers file, you have your other mail address: "Matej Vido <matejvido@gmail.com>", do you want to update it? > --- > drivers/net/szedata2/rte_eth_szedata2.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/net/szedata2/rte_eth_szedata2.h b/drivers/net/szedata2/rte_eth_szedata2.h > index b58adb6..afe8a38 100644 > --- a/drivers/net/szedata2/rte_eth_szedata2.h > +++ b/drivers/net/szedata2/rte_eth_szedata2.h > @@ -192,7 +192,7 @@ struct szedata { > } > > #define SZEDATA2_PCI_RESOURCE_PTR(rsc, offset, type) \ > - ((type)((uint8_t *)(rsc)->addr) + (offset)) > + ((type)(((uint8_t *)(rsc)->addr) + (offset))) Although output will be same, (in all uses, type is a pointer), this seems the intention, so: Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com> btw, following will do same, right, not sure if it is better: ((type)(rsc)->addr + (offset)) > > enum szedata2_link_speed { > SZEDATA2_LINK_SPEED_DEFAULT = 0, > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [dpdk-dev] [PATCH] net/szedata2: fix incorrect device memory access 2017-01-24 11:58 ` Ferruh Yigit @ 2017-01-24 14:02 ` Matej Vido 2017-01-24 14:17 ` Ferruh Yigit 2017-01-24 15:11 ` Ferruh Yigit 0 siblings, 2 replies; 10+ messages in thread From: Matej Vido @ 2017-01-24 14:02 UTC (permalink / raw) To: Ferruh Yigit; +Cc: dev On 24.01.2017 12:58, Ferruh Yigit wrote: > On 1/24/2017 10:49 AM, Matej Vido wrote: >> Fixes: 8acba705b119 ("net/szedata2: localize handling of PCI resources") >> >> Signed-off-by: Matej Vido <vido@cesnet.cz> > Unrelated from this patch, in maintainers file, you have your other mail > address: "Matej Vido <matejvido@gmail.com>", do you want to update it? Hi Ferruh, yes, I will send the patch. > >> --- >> drivers/net/szedata2/rte_eth_szedata2.h | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/net/szedata2/rte_eth_szedata2.h b/drivers/net/szedata2/rte_eth_szedata2.h >> index b58adb6..afe8a38 100644 >> --- a/drivers/net/szedata2/rte_eth_szedata2.h >> +++ b/drivers/net/szedata2/rte_eth_szedata2.h >> @@ -192,7 +192,7 @@ struct szedata { >> } >> >> #define SZEDATA2_PCI_RESOURCE_PTR(rsc, offset, type) \ >> - ((type)((uint8_t *)(rsc)->addr) + (offset)) >> + ((type)(((uint8_t *)(rsc)->addr) + (offset))) > Although output will be same, (in all uses, type is a pointer), this > seems the intention, so: > > Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com> > > btw, following will do same, right, not sure if it is better: > ((type)(rsc)->addr + (offset)) This is also wrong. The intention of the macro is to add an offset to the base address and typecast the result. Regards, Matej > >> >> enum szedata2_link_speed { >> SZEDATA2_LINK_SPEED_DEFAULT = 0, >> ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [dpdk-dev] [PATCH] net/szedata2: fix incorrect device memory access 2017-01-24 14:02 ` Matej Vido @ 2017-01-24 14:17 ` Ferruh Yigit 2017-01-24 15:11 ` Ferruh Yigit 1 sibling, 0 replies; 10+ messages in thread From: Ferruh Yigit @ 2017-01-24 14:17 UTC (permalink / raw) To: Matej Vido; +Cc: dev On 1/24/2017 2:02 PM, Matej Vido wrote: > On 24.01.2017 12:58, Ferruh Yigit wrote: >> On 1/24/2017 10:49 AM, Matej Vido wrote: >>> Fixes: 8acba705b119 ("net/szedata2: localize handling of PCI resources") >>> >>> Signed-off-by: Matej Vido <vido@cesnet.cz> >> Unrelated from this patch, in maintainers file, you have your other mail >> address: "Matej Vido <matejvido@gmail.com>", do you want to update it? > Hi Ferruh, > > yes, I will send the patch. >> >>> --- >>> drivers/net/szedata2/rte_eth_szedata2.h | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/drivers/net/szedata2/rte_eth_szedata2.h b/drivers/net/szedata2/rte_eth_szedata2.h >>> index b58adb6..afe8a38 100644 >>> --- a/drivers/net/szedata2/rte_eth_szedata2.h >>> +++ b/drivers/net/szedata2/rte_eth_szedata2.h >>> @@ -192,7 +192,7 @@ struct szedata { >>> } >>> >>> #define SZEDATA2_PCI_RESOURCE_PTR(rsc, offset, type) \ >>> - ((type)((uint8_t *)(rsc)->addr) + (offset)) >>> + ((type)(((uint8_t *)(rsc)->addr) + (offset))) >> Although output will be same, (in all uses, type is a pointer), this >> seems the intention, so: >> >> Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com> >> >> btw, following will do same, right, not sure if it is better: >> ((type)(rsc)->addr + (offset)) > This is also wrong. The intention of the macro is to add an offset to > the base address and typecast the result. Right, again this will give same output when "type" is pointer, but wrong for described intention. > > Regards, > Matej >> >>> >>> enum szedata2_link_speed { >>> SZEDATA2_LINK_SPEED_DEFAULT = 0, >>> > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [dpdk-dev] [PATCH] net/szedata2: fix incorrect device memory access 2017-01-24 14:02 ` Matej Vido 2017-01-24 14:17 ` Ferruh Yigit @ 2017-01-24 15:11 ` Ferruh Yigit 2017-01-24 15:55 ` Matej Vido 1 sibling, 1 reply; 10+ messages in thread From: Ferruh Yigit @ 2017-01-24 15:11 UTC (permalink / raw) To: Matej Vido; +Cc: dev On 1/24/2017 2:02 PM, Matej Vido wrote: > On 24.01.2017 12:58, Ferruh Yigit wrote: >> On 1/24/2017 10:49 AM, Matej Vido wrote: >>> Fixes: 8acba705b119 ("net/szedata2: localize handling of PCI resources") >>> >>> Signed-off-by: Matej Vido <vido@cesnet.cz> >> Unrelated from this patch, in maintainers file, you have your other mail >> address: "Matej Vido <matejvido@gmail.com>", do you want to update it? > Hi Ferruh, > > yes, I will send the patch. >> >>> --- >>> drivers/net/szedata2/rte_eth_szedata2.h | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/drivers/net/szedata2/rte_eth_szedata2.h b/drivers/net/szedata2/rte_eth_szedata2.h >>> index b58adb6..afe8a38 100644 >>> --- a/drivers/net/szedata2/rte_eth_szedata2.h >>> +++ b/drivers/net/szedata2/rte_eth_szedata2.h >>> @@ -192,7 +192,7 @@ struct szedata { >>> } >>> >>> #define SZEDATA2_PCI_RESOURCE_PTR(rsc, offset, type) \ >>> - ((type)((uint8_t *)(rsc)->addr) + (offset)) >>> + ((type)(((uint8_t *)(rsc)->addr) + (offset))) >> Although output will be same, (in all uses, type is a pointer), Of course won't be same, please forget about it J, I am confused. So these two differs a lot, taking into account that offset numbers used are big numbers (0x8000..), it should be accessing very unrelated addresses. So how this was working before? > this >> seems the intention, so: >> >> Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com> >> >> btw, following will do same, right, not sure if it is better: >> ((type)(rsc)->addr + (offset)) > This is also wrong. The intention of the macro is to add an offset to > the base address and typecast the result. > > Regards, > Matej >> >>> >>> enum szedata2_link_speed { >>> SZEDATA2_LINK_SPEED_DEFAULT = 0, >>> > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [dpdk-dev] [PATCH] net/szedata2: fix incorrect device memory access 2017-01-24 15:11 ` Ferruh Yigit @ 2017-01-24 15:55 ` Matej Vido 2017-01-24 16:03 ` Ferruh Yigit 0 siblings, 1 reply; 10+ messages in thread From: Matej Vido @ 2017-01-24 15:55 UTC (permalink / raw) To: Ferruh Yigit; +Cc: dev On 24.01.2017 16:11, Ferruh Yigit wrote: > On 1/24/2017 2:02 PM, Matej Vido wrote: >> On 24.01.2017 12:58, Ferruh Yigit wrote: >>> On 1/24/2017 10:49 AM, Matej Vido wrote: >>>> Fixes: 8acba705b119 ("net/szedata2: localize handling of PCI resources") >>>> >>>> Signed-off-by: Matej Vido <vido@cesnet.cz> >>> Unrelated from this patch, in maintainers file, you have your other mail >>> address: "Matej Vido <matejvido@gmail.com>", do you want to update it? >> Hi Ferruh, >> >> yes, I will send the patch. >>>> --- >>>> drivers/net/szedata2/rte_eth_szedata2.h | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/net/szedata2/rte_eth_szedata2.h b/drivers/net/szedata2/rte_eth_szedata2.h >>>> index b58adb6..afe8a38 100644 >>>> --- a/drivers/net/szedata2/rte_eth_szedata2.h >>>> +++ b/drivers/net/szedata2/rte_eth_szedata2.h >>>> @@ -192,7 +192,7 @@ struct szedata { >>>> } >>>> >>>> #define SZEDATA2_PCI_RESOURCE_PTR(rsc, offset, type) \ >>>> - ((type)((uint8_t *)(rsc)->addr) + (offset)) >>>> + ((type)(((uint8_t *)(rsc)->addr) + (offset))) >>> Although output will be same, (in all uses, type is a pointer), > Of course won't be same, please forget about it J, I am confused. > > So these two differs a lot, taking into account that offset numbers used > are big numbers (0x8000..), it should be accessing very unrelated addresses. > > So how this was working before? The macro was fine before the patch [1]. It hasn't been working since the acceptance of that patch, but I didn't manage to test the functionality until now. [1] http://dpdk.org/ml/archives/dev/2016-December/053241.html Regards, Matej > >> this >>> seems the intention, so: >>> >>> Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com> >>> >>> btw, following will do same, right, not sure if it is better: >>> ((type)(rsc)->addr + (offset)) >> This is also wrong. The intention of the macro is to add an offset to >> the base address and typecast the result. >> >> Regards, >> Matej >>>> >>>> enum szedata2_link_speed { >>>> SZEDATA2_LINK_SPEED_DEFAULT = 0, >>>> ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [dpdk-dev] [PATCH] net/szedata2: fix incorrect device memory access 2017-01-24 15:55 ` Matej Vido @ 2017-01-24 16:03 ` Ferruh Yigit 0 siblings, 0 replies; 10+ messages in thread From: Ferruh Yigit @ 2017-01-24 16:03 UTC (permalink / raw) To: Matej Vido; +Cc: dev On 1/24/2017 3:55 PM, Matej Vido wrote: > On 24.01.2017 16:11, Ferruh Yigit wrote: >> On 1/24/2017 2:02 PM, Matej Vido wrote: >>> On 24.01.2017 12:58, Ferruh Yigit wrote: >>>> On 1/24/2017 10:49 AM, Matej Vido wrote: >>>>> Fixes: 8acba705b119 ("net/szedata2: localize handling of PCI resources") >>>>> >>>>> Signed-off-by: Matej Vido <vido@cesnet.cz> >>>> Unrelated from this patch, in maintainers file, you have your other mail >>>> address: "Matej Vido <matejvido@gmail.com>", do you want to update it? >>> Hi Ferruh, >>> >>> yes, I will send the patch. >>>>> --- >>>>> drivers/net/szedata2/rte_eth_szedata2.h | 2 +- >>>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>>> >>>>> diff --git a/drivers/net/szedata2/rte_eth_szedata2.h b/drivers/net/szedata2/rte_eth_szedata2.h >>>>> index b58adb6..afe8a38 100644 >>>>> --- a/drivers/net/szedata2/rte_eth_szedata2.h >>>>> +++ b/drivers/net/szedata2/rte_eth_szedata2.h >>>>> @@ -192,7 +192,7 @@ struct szedata { >>>>> } >>>>> >>>>> #define SZEDATA2_PCI_RESOURCE_PTR(rsc, offset, type) \ >>>>> - ((type)((uint8_t *)(rsc)->addr) + (offset)) >>>>> + ((type)(((uint8_t *)(rsc)->addr) + (offset))) >>>> Although output will be same, (in all uses, type is a pointer), >> Of course won't be same, please forget about it J, I am confused. >> >> So these two differs a lot, taking into account that offset numbers used >> are big numbers (0x8000..), it should be accessing very unrelated addresses. >> >> So how this was working before? > > The macro was fine before the patch [1]. It hasn't been working since > the acceptance of that patch, but I didn't manage to test the > functionality until now. I see, thanks for clarification, seems broken relatively new, and thanks for fixing. > > [1] http://dpdk.org/ml/archives/dev/2016-December/053241.html > > Regards, > Matej > >> >>> this >>>> seems the intention, so: >>>> >>>> Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com> >>>> >>>> btw, following will do same, right, not sure if it is better: >>>> ((type)(rsc)->addr + (offset)) >>> This is also wrong. The intention of the macro is to add an offset to >>> the base address and typecast the result. >>> >>> Regards, >>> Matej >>>>> >>>>> enum szedata2_link_speed { >>>>> SZEDATA2_LINK_SPEED_DEFAULT = 0, >>>>> > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [dpdk-dev] [PATCH] net/szedata2: fix incorrect device memory access 2017-01-24 10:49 [dpdk-dev] [PATCH] net/szedata2: fix incorrect device memory access Matej Vido 2017-01-24 11:58 ` Ferruh Yigit @ 2017-01-24 15:24 ` Ferruh Yigit 2017-01-24 16:05 ` Matej Vido 1 sibling, 1 reply; 10+ messages in thread From: Ferruh Yigit @ 2017-01-24 15:24 UTC (permalink / raw) To: Matej Vido, dev, dpdk stable On 1/24/2017 10:49 AM, Matej Vido wrote: > Fixes: 8acba705b119 ("net/szedata2: localize handling of PCI resources") > > Signed-off-by: Matej Vido <vido@cesnet.cz> Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com> Cc: stable@dpdk.org Applied to dpdk-next-net/master, thanks. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [dpdk-dev] [PATCH] net/szedata2: fix incorrect device memory access 2017-01-24 15:24 ` Ferruh Yigit @ 2017-01-24 16:05 ` Matej Vido 2017-01-24 16:29 ` Ferruh Yigit 0 siblings, 1 reply; 10+ messages in thread From: Matej Vido @ 2017-01-24 16:05 UTC (permalink / raw) To: Ferruh Yigit; +Cc: dev, dpdk stable On 24.01.2017 16:24, Ferruh Yigit wrote: > On 1/24/2017 10:49 AM, Matej Vido wrote: >> Fixes: 8acba705b119 ("net/szedata2: localize handling of PCI resources") >> >> Signed-off-by: Matej Vido <vido@cesnet.cz> > Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com> > > Cc: stable@dpdk.org > > Applied to dpdk-next-net/master, thanks. I'm not sure about the policy regarding the stable releases, but I think that this patch doesn't belong to stable as the bug was introduced and also fixed during the current 17.02 window. Or am I wrong? Thanks, Matej ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [dpdk-dev] [PATCH] net/szedata2: fix incorrect device memory access 2017-01-24 16:05 ` Matej Vido @ 2017-01-24 16:29 ` Ferruh Yigit 0 siblings, 0 replies; 10+ messages in thread From: Ferruh Yigit @ 2017-01-24 16:29 UTC (permalink / raw) To: Matej Vido; +Cc: dev, dpdk stable On 1/24/2017 4:05 PM, Matej Vido wrote: > On 24.01.2017 16:24, Ferruh Yigit wrote: >> On 1/24/2017 10:49 AM, Matej Vido wrote: >>> Fixes: 8acba705b119 ("net/szedata2: localize handling of PCI resources") >>> >>> Signed-off-by: Matej Vido <vido@cesnet.cz> >> Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com> >> >> Cc: stable@dpdk.org >> >> Applied to dpdk-next-net/master, thanks. > I'm not sure about the policy regarding the stable releases, but I think > that this patch doesn't belong to stable as the bug was introduced and > also fixed during the current 17.02 window. Or am I wrong? Yes you are right, I missed that issue introduced in this release, I removed stable tag from commit log. Thanks, ferruh > > Thanks, > Matej > ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2017-01-24 16:29 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-01-24 10:49 [dpdk-dev] [PATCH] net/szedata2: fix incorrect device memory access Matej Vido 2017-01-24 11:58 ` Ferruh Yigit 2017-01-24 14:02 ` Matej Vido 2017-01-24 14:17 ` Ferruh Yigit 2017-01-24 15:11 ` Ferruh Yigit 2017-01-24 15:55 ` Matej Vido 2017-01-24 16:03 ` Ferruh Yigit 2017-01-24 15:24 ` Ferruh Yigit 2017-01-24 16:05 ` Matej Vido 2017-01-24 16:29 ` Ferruh Yigit
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).