* [PATCH] net/gve: fix bug in verify driver compatibility @ 2023-06-01 3:15 Rushil Gupta 2023-06-01 4:49 ` Rushil Gupta 0 siblings, 1 reply; 10+ messages in thread From: Rushil Gupta @ 2023-06-01 3:15 UTC (permalink / raw) To: qi.z.zhang, ferruh.yigit; +Cc: junfeng.guo, dev, Rushil Gupta gVNIC requires physical address to be passed in the adminq command. This was initially rightly pointed by ferruh.yigit@. Fixed by passing 'driver_info_mem->iova'. Signed-off-by: Rushil Gupta <rushilg@google.com> --- drivers/net/gve/gve_ethdev.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/net/gve/gve_ethdev.c b/drivers/net/gve/gve_ethdev.c index 2c1e73d07a..1c5ce930a4 100644 --- a/drivers/net/gve/gve_ethdev.c +++ b/drivers/net/gve/gve_ethdev.c @@ -276,7 +276,8 @@ gve_verify_driver_compatibility(struct gve_priv *priv) (char *)driver_info->os_version_str2); err = gve_adminq_verify_driver_compatibility(priv, - sizeof(struct gve_driver_info), (dma_addr_t)driver_info); + sizeof(struct gve_driver_info), + (dma_addr_t)driver_info_mem->iova); /* It's ok if the device doesn't support this */ if (err == -EOPNOTSUPP) err = 0; -- 2.41.0.rc2.161.g9c6817b8e7-goog ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] net/gve: fix bug in verify driver compatibility 2023-06-01 3:15 [PATCH] net/gve: fix bug in verify driver compatibility Rushil Gupta @ 2023-06-01 4:49 ` Rushil Gupta 2023-06-01 6:44 ` Guo, Junfeng 2023-06-01 8:26 ` Ferruh Yigit 0 siblings, 2 replies; 10+ messages in thread From: Rushil Gupta @ 2023-06-01 4:49 UTC (permalink / raw) To: qi.z.zhang, ferruh.yigit; +Cc: junfeng.guo, dev, Rushil Gupta gVNIC requires physical address to be passed in the adminq command. This was initially rightly pointed by ferruh.yigit@. Fixed by passing 'driver_info_mem->iova'. Signed-off-by: Rushil Gupta <rushilg@google.com> --- drivers/net/gve/gve_ethdev.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/net/gve/gve_ethdev.c b/drivers/net/gve/gve_ethdev.c index 2c1e73d07a..aa75abe102 100644 --- a/drivers/net/gve/gve_ethdev.c +++ b/drivers/net/gve/gve_ethdev.c @@ -276,7 +276,8 @@ gve_verify_driver_compatibility(struct gve_priv *priv) (char *)driver_info->os_version_str2); err = gve_adminq_verify_driver_compatibility(priv, - sizeof(struct gve_driver_info), (dma_addr_t)driver_info); + sizeof(struct gve_driver_info), + (dma_addr_t)driver_info_mem->iova); /* It's ok if the device doesn't support this */ if (err == -EOPNOTSUPP) err = 0; -- 2.41.0.rc2.161.g9c6817b8e7-goog ^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH] net/gve: fix bug in verify driver compatibility 2023-06-01 4:49 ` Rushil Gupta @ 2023-06-01 6:44 ` Guo, Junfeng 2023-06-01 8:35 ` Ferruh Yigit 2023-06-01 8:26 ` Ferruh Yigit 1 sibling, 1 reply; 10+ messages in thread From: Guo, Junfeng @ 2023-06-01 6:44 UTC (permalink / raw) To: Rushil Gupta, Zhang, Qi Z, ferruh.yigit; +Cc: dev > -----Original Message----- > From: Rushil Gupta <rushilg@google.com> > Sent: Thursday, June 1, 2023 12:50 > To: Zhang, Qi Z <qi.z.zhang@intel.com>; ferruh.yigit@amd.com > Cc: Guo, Junfeng <junfeng.guo@intel.com>; dev@dpdk.org; Rushil Gupta > <rushilg@google.com> > Subject: [PATCH] net/gve: fix bug in verify driver compatibility > > gVNIC requires physical address to be passed in the adminq command. > This was initially rightly pointed by ferruh.yigit@. > Fixed by passing 'driver_info_mem->iova'. For bug fixing, it is required to reference the id of the commit which introduced the bug. You can generate the required lines using the following git alias: ''' git config alias.fixline "log -1 --abbrev=12 --format='Fixes: %h (\"%s\")%nCc: %ae'" ''' and then put the output of ''' git fixline <SHA> ''' into the commit message. And also be CCed to the stable@dpdk.org mailing list. Thanks! Regards, Junfeng > > Signed-off-by: Rushil Gupta <rushilg@google.com> > --- > drivers/net/gve/gve_ethdev.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/gve/gve_ethdev.c b/drivers/net/gve/gve_ethdev.c > index 2c1e73d07a..aa75abe102 100644 > --- a/drivers/net/gve/gve_ethdev.c > +++ b/drivers/net/gve/gve_ethdev.c > @@ -276,7 +276,8 @@ gve_verify_driver_compatibility(struct gve_priv > *priv) > (char *)driver_info->os_version_str2); > > err = gve_adminq_verify_driver_compatibility(priv, > - sizeof(struct gve_driver_info), (dma_addr_t)driver_info); > + sizeof(struct gve_driver_info), > + (dma_addr_t)driver_info_mem->iova); > /* It's ok if the device doesn't support this */ > if (err == -EOPNOTSUPP) > err = 0; > -- > 2.41.0.rc2.161.g9c6817b8e7-goog ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] net/gve: fix bug in verify driver compatibility 2023-06-01 6:44 ` Guo, Junfeng @ 2023-06-01 8:35 ` Ferruh Yigit 2023-06-01 8:58 ` Guo, Junfeng 0 siblings, 1 reply; 10+ messages in thread From: Ferruh Yigit @ 2023-06-01 8:35 UTC (permalink / raw) To: Guo, Junfeng, Rushil Gupta, Zhang, Qi Z; +Cc: dev On 6/1/2023 7:44 AM, Guo, Junfeng wrote: > > >> -----Original Message----- >> From: Rushil Gupta <rushilg@google.com> >> Sent: Thursday, June 1, 2023 12:50 >> To: Zhang, Qi Z <qi.z.zhang@intel.com>; ferruh.yigit@amd.com >> Cc: Guo, Junfeng <junfeng.guo@intel.com>; dev@dpdk.org; Rushil Gupta >> <rushilg@google.com> >> Subject: [PATCH] net/gve: fix bug in verify driver compatibility >> >> gVNIC requires physical address to be passed in the adminq command. >> This was initially rightly pointed by ferruh.yigit@. >> Fixed by passing 'driver_info_mem->iova'. > > For bug fixing, it is required to reference the id of the commit which introduced the bug. > You can generate the required lines using the following git alias: > ''' git config alias.fixline "log -1 --abbrev=12 --format='Fixes: %h (\"%s\")%nCc: %ae'" ''' > and then put the output of ''' git fixline <SHA> ''' into the commit message. > > And also be CCed to the stable@dpdk.org mailing list. Thanks! > > Regards, > Junfeng > Hi Junfeng, You are right, but for this case patch is not pulled to main repo yet, and change is trivial, so I will fix it in next-net. ^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH] net/gve: fix bug in verify driver compatibility 2023-06-01 8:35 ` Ferruh Yigit @ 2023-06-01 8:58 ` Guo, Junfeng 0 siblings, 0 replies; 10+ messages in thread From: Guo, Junfeng @ 2023-06-01 8:58 UTC (permalink / raw) To: Ferruh Yigit, Rushil Gupta, Zhang, Qi Z; +Cc: dev > -----Original Message----- > From: Ferruh Yigit <ferruh.yigit@amd.com> > Sent: Thursday, June 1, 2023 16:35 > To: Guo, Junfeng <junfeng.guo@intel.com>; Rushil Gupta > <rushilg@google.com>; Zhang, Qi Z <qi.z.zhang@intel.com> > Cc: dev@dpdk.org > Subject: Re: [PATCH] net/gve: fix bug in verify driver compatibility > > On 6/1/2023 7:44 AM, Guo, Junfeng wrote: > > > > > >> -----Original Message----- > >> From: Rushil Gupta <rushilg@google.com> > >> Sent: Thursday, June 1, 2023 12:50 > >> To: Zhang, Qi Z <qi.z.zhang@intel.com>; ferruh.yigit@amd.com > >> Cc: Guo, Junfeng <junfeng.guo@intel.com>; dev@dpdk.org; Rushil > Gupta > >> <rushilg@google.com> > >> Subject: [PATCH] net/gve: fix bug in verify driver compatibility > >> > >> gVNIC requires physical address to be passed in the adminq command. > >> This was initially rightly pointed by ferruh.yigit@. > >> Fixed by passing 'driver_info_mem->iova'. > > > > For bug fixing, it is required to reference the id of the commit which > introduced the bug. > > You can generate the required lines using the following git alias: > > ''' git config alias.fixline "log -1 --abbrev=12 --format='Fixes: %h > (\"%s\")%nCc: %ae'" ''' > > and then put the output of ''' git fixline <SHA> ''' into the commit > message. > > > > And also be CCed to the stable@dpdk.org mailing list. Thanks! > > > > Regards, > > Junfeng > > > > Hi Junfeng, > > You are right, but for this case patch is not pulled to main repo yet, > and change is trivial, so I will fix it in next-net. Sure, thanks for the efforts! ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] net/gve: fix bug in verify driver compatibility 2023-06-01 4:49 ` Rushil Gupta 2023-06-01 6:44 ` Guo, Junfeng @ 2023-06-01 8:26 ` Ferruh Yigit 2023-06-01 10:24 ` Ferruh Yigit 1 sibling, 1 reply; 10+ messages in thread From: Ferruh Yigit @ 2023-06-01 8:26 UTC (permalink / raw) To: Rushil Gupta, qi.z.zhang; +Cc: junfeng.guo, dev On 6/1/2023 5:49 AM, Rushil Gupta wrote: > gVNIC requires physical address to be passed in the adminq command. > This was initially rightly pointed by ferruh.yigit@. > Fixed by passing 'driver_info_mem->iova'. > > Signed-off-by: Rushil Gupta <rushilg@google.com> > --- > drivers/net/gve/gve_ethdev.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/gve/gve_ethdev.c b/drivers/net/gve/gve_ethdev.c > index 2c1e73d07a..aa75abe102 100644 > --- a/drivers/net/gve/gve_ethdev.c > +++ b/drivers/net/gve/gve_ethdev.c > @@ -276,7 +276,8 @@ gve_verify_driver_compatibility(struct gve_priv *priv) > (char *)driver_info->os_version_str2); > > err = gve_adminq_verify_driver_compatibility(priv, > - sizeof(struct gve_driver_info), (dma_addr_t)driver_info); > + sizeof(struct gve_driver_info), > + (dma_addr_t)driver_info_mem->iova); > Yep, this was my point, let me squashed onto original patch in next-net. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] net/gve: fix bug in verify driver compatibility 2023-06-01 8:26 ` Ferruh Yigit @ 2023-06-01 10:24 ` Ferruh Yigit 2023-06-01 16:32 ` Rushil Gupta 0 siblings, 1 reply; 10+ messages in thread From: Ferruh Yigit @ 2023-06-01 10:24 UTC (permalink / raw) To: Rushil Gupta, qi.z.zhang; +Cc: junfeng.guo, dev On 6/1/2023 9:26 AM, Ferruh Yigit wrote: > On 6/1/2023 5:49 AM, Rushil Gupta wrote: >> gVNIC requires physical address to be passed in the adminq command. >> This was initially rightly pointed by ferruh.yigit@. >> Fixed by passing 'driver_info_mem->iova'. >> >> Signed-off-by: Rushil Gupta <rushilg@google.com> >> --- >> drivers/net/gve/gve_ethdev.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/net/gve/gve_ethdev.c b/drivers/net/gve/gve_ethdev.c >> index 2c1e73d07a..aa75abe102 100644 >> --- a/drivers/net/gve/gve_ethdev.c >> +++ b/drivers/net/gve/gve_ethdev.c >> @@ -276,7 +276,8 @@ gve_verify_driver_compatibility(struct gve_priv *priv) >> (char *)driver_info->os_version_str2); >> >> err = gve_adminq_verify_driver_compatibility(priv, >> - sizeof(struct gve_driver_info), (dma_addr_t)driver_info); >> + sizeof(struct gve_driver_info), >> + (dma_addr_t)driver_info_mem->iova); >> > > Yep, this was my point, let me squashed onto original patch in next-net. > > Squashed into relevant commit in next-net, thanks. Can you please verify latest code in next-net? ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] net/gve: fix bug in verify driver compatibility 2023-06-01 10:24 ` Ferruh Yigit @ 2023-06-01 16:32 ` Rushil Gupta 2023-06-02 14:49 ` Ferruh Yigit 0 siblings, 1 reply; 10+ messages in thread From: Rushil Gupta @ 2023-06-01 16:32 UTC (permalink / raw) To: Ferruh Yigit; +Cc: qi.z.zhang, junfeng.guo, dev Thanks for the quick response! Just for my own knowledge, what Junfeng described is the process to fix the bug if a bug is present in the main dpdk repo? On Thu, Jun 1, 2023 at 3:24 AM Ferruh Yigit <ferruh.yigit@amd.com> wrote: > > On 6/1/2023 9:26 AM, Ferruh Yigit wrote: > > On 6/1/2023 5:49 AM, Rushil Gupta wrote: > >> gVNIC requires physical address to be passed in the adminq command. > >> This was initially rightly pointed by ferruh.yigit@. > >> Fixed by passing 'driver_info_mem->iova'. > >> > >> Signed-off-by: Rushil Gupta <rushilg@google.com> > >> --- > >> drivers/net/gve/gve_ethdev.c | 3 ++- > >> 1 file changed, 2 insertions(+), 1 deletion(-) > >> > >> diff --git a/drivers/net/gve/gve_ethdev.c b/drivers/net/gve/gve_ethdev.c > >> index 2c1e73d07a..aa75abe102 100644 > >> --- a/drivers/net/gve/gve_ethdev.c > >> +++ b/drivers/net/gve/gve_ethdev.c > >> @@ -276,7 +276,8 @@ gve_verify_driver_compatibility(struct gve_priv *priv) > >> (char *)driver_info->os_version_str2); > >> > >> err = gve_adminq_verify_driver_compatibility(priv, > >> - sizeof(struct gve_driver_info), (dma_addr_t)driver_info); > >> + sizeof(struct gve_driver_info), > >> + (dma_addr_t)driver_info_mem->iova); > >> > > > > Yep, this was my point, let me squashed onto original patch in next-net. > > > > > > Squashed into relevant commit in next-net, thanks. > Can you please verify latest code in next-net? ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] net/gve: fix bug in verify driver compatibility 2023-06-01 16:32 ` Rushil Gupta @ 2023-06-02 14:49 ` Ferruh Yigit 2023-06-02 21:44 ` Rushil Gupta 0 siblings, 1 reply; 10+ messages in thread From: Ferruh Yigit @ 2023-06-02 14:49 UTC (permalink / raw) To: Rushil Gupta; +Cc: qi.z.zhang, junfeng.guo, dev On 6/1/2023 5:32 PM, Rushil Gupta wrote: > Thanks for the quick response! > Just for my own knowledge, what Junfeng described is the process to > fix the bug if a bug is present in the main dpdk repo? > Correct. As main dpdk repo doesn't rewrite git history, any bug there needs to be fixed with an incremental commit, and fix commit should have some metadata to describe what it is fixing (this is to help LTS maintainers and developers tracing down a change). But sub-trees (like next-net) allowed to rewrite git history, so allowed to squash/rebase commits when it is more practical to do so as this case. But changing directly in the git repo has its problems, like tracing, justifying source of the change and missing consensus etc. So, even it is squashed, the patch should be publicly available in the mail list first. For developers it is best to send incremental patch as if it will be merged as incremental commit, but can request a squash if the commit still not pulled by main repo, or maintainer can prefer to squash the fix to original commit, to reduce the churn & process overhead. > On Thu, Jun 1, 2023 at 3:24 AM Ferruh Yigit <ferruh.yigit@amd.com> wrote: >> >> On 6/1/2023 9:26 AM, Ferruh Yigit wrote: >>> On 6/1/2023 5:49 AM, Rushil Gupta wrote: >>>> gVNIC requires physical address to be passed in the adminq command. >>>> This was initially rightly pointed by ferruh.yigit@. >>>> Fixed by passing 'driver_info_mem->iova'. >>>> >>>> Signed-off-by: Rushil Gupta <rushilg@google.com> >>>> --- >>>> drivers/net/gve/gve_ethdev.c | 3 ++- >>>> 1 file changed, 2 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/net/gve/gve_ethdev.c b/drivers/net/gve/gve_ethdev.c >>>> index 2c1e73d07a..aa75abe102 100644 >>>> --- a/drivers/net/gve/gve_ethdev.c >>>> +++ b/drivers/net/gve/gve_ethdev.c >>>> @@ -276,7 +276,8 @@ gve_verify_driver_compatibility(struct gve_priv *priv) >>>> (char *)driver_info->os_version_str2); >>>> >>>> err = gve_adminq_verify_driver_compatibility(priv, >>>> - sizeof(struct gve_driver_info), (dma_addr_t)driver_info); >>>> + sizeof(struct gve_driver_info), >>>> + (dma_addr_t)driver_info_mem->iova); >>>> >>> >>> Yep, this was my point, let me squashed onto original patch in next-net. >>> >>> >> >> Squashed into relevant commit in next-net, thanks. >> Can you please verify latest code in next-net? ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] net/gve: fix bug in verify driver compatibility 2023-06-02 14:49 ` Ferruh Yigit @ 2023-06-02 21:44 ` Rushil Gupta 0 siblings, 0 replies; 10+ messages in thread From: Rushil Gupta @ 2023-06-02 21:44 UTC (permalink / raw) To: Ferruh Yigit; +Cc: qi.z.zhang, junfeng.guo, dev Thanks for the clarification and for squashing the bug into the main commit. I just tested the net-next branch and it seems to log the right message on the device. On Fri, Jun 2, 2023 at 7:49 AM Ferruh Yigit <ferruh.yigit@amd.com> wrote: > > On 6/1/2023 5:32 PM, Rushil Gupta wrote: > > Thanks for the quick response! > > Just for my own knowledge, what Junfeng described is the process to > > fix the bug if a bug is present in the main dpdk repo? > > > > Correct. > > As main dpdk repo doesn't rewrite git history, any bug there needs to be > fixed with an incremental commit, and fix commit should have some > metadata to describe what it is fixing (this is to help LTS maintainers > and developers tracing down a change). > > But sub-trees (like next-net) allowed to rewrite git history, so allowed > to squash/rebase commits when it is more practical to do so as this case. > But changing directly in the git repo has its problems, like tracing, > justifying source of the change and missing consensus etc. So, even it > is squashed, the patch should be publicly available in the mail list first. > > For developers it is best to send incremental patch as if it will be > merged as incremental commit, but can request a squash if the commit > still not pulled by main repo, or maintainer can prefer to squash the > fix to original commit, to reduce the churn & process overhead. > > > On Thu, Jun 1, 2023 at 3:24 AM Ferruh Yigit <ferruh.yigit@amd.com> wrote: > >> > >> On 6/1/2023 9:26 AM, Ferruh Yigit wrote: > >>> On 6/1/2023 5:49 AM, Rushil Gupta wrote: > >>>> gVNIC requires physical address to be passed in the adminq command. > >>>> This was initially rightly pointed by ferruh.yigit@. > >>>> Fixed by passing 'driver_info_mem->iova'. > >>>> > >>>> Signed-off-by: Rushil Gupta <rushilg@google.com> > >>>> --- > >>>> drivers/net/gve/gve_ethdev.c | 3 ++- > >>>> 1 file changed, 2 insertions(+), 1 deletion(-) > >>>> > >>>> diff --git a/drivers/net/gve/gve_ethdev.c b/drivers/net/gve/gve_ethdev.c > >>>> index 2c1e73d07a..aa75abe102 100644 > >>>> --- a/drivers/net/gve/gve_ethdev.c > >>>> +++ b/drivers/net/gve/gve_ethdev.c > >>>> @@ -276,7 +276,8 @@ gve_verify_driver_compatibility(struct gve_priv *priv) > >>>> (char *)driver_info->os_version_str2); > >>>> > >>>> err = gve_adminq_verify_driver_compatibility(priv, > >>>> - sizeof(struct gve_driver_info), (dma_addr_t)driver_info); > >>>> + sizeof(struct gve_driver_info), > >>>> + (dma_addr_t)driver_info_mem->iova); > >>>> > >>> > >>> Yep, this was my point, let me squashed onto original patch in next-net. > >>> > >>> > >> > >> Squashed into relevant commit in next-net, thanks. > >> Can you please verify latest code in next-net? > ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2023-06-02 21:44 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-06-01 3:15 [PATCH] net/gve: fix bug in verify driver compatibility Rushil Gupta 2023-06-01 4:49 ` Rushil Gupta 2023-06-01 6:44 ` Guo, Junfeng 2023-06-01 8:35 ` Ferruh Yigit 2023-06-01 8:58 ` Guo, Junfeng 2023-06-01 8:26 ` Ferruh Yigit 2023-06-01 10:24 ` Ferruh Yigit 2023-06-01 16:32 ` Rushil Gupta 2023-06-02 14:49 ` Ferruh Yigit 2023-06-02 21:44 ` Rushil Gupta
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).