Re: [PATCH] net/gve: fix bug in verify driver compatibility

[Ferruh Yigit <ferruh.yigit@amd.com>]

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?