Re: [PATCH] net/tap: Bug fix to populate fds in secondary process

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

On 1/21/2022 4:29 AM, Kumara Parameshwaran wrote:
> From: Kumara Parameshwaran<kparameshwar@vmware.com>
> 
> When a tap device is hotplugged to primary process which in turn
> adds the device to all secondary process, the secondary process
> does a tap_mp_attach_queues, but the fds are not populated in
> the primary during the probe they are populated during the queue_setup,
> added a fix to sync the queues during rte_eth_dev_start
> 
> Signed-off-by: Kumara Parameshwaran<kparameshwar@vmware.com>
> ---
>   drivers/net/tap/rte_eth_tap.c | 196 +++++++++++++---------------------
>   lib/ethdev/rte_ethdev.c       |  11 ++
>   lib/ethdev/rte_ethdev.h       |  17 +++
>   lib/ethdev/version.map        |   2 +
>   4 files changed, 102 insertions(+), 124 deletions(-)

Hi Kumara,

I see you have sent multiple version of the patch, but it is very hard to follow
them in the way you did it, can you please check:
https://doc.dpdk.org/guides/contributing/patches.html


And briefly:
- Please put a version information in the patch title, like:
   [PATCH v2] ....
   There are "git format-patch" or "git send-email" argument to do this easily

- Keep a changelog in the patch, as a note (after "---" in the commit log),
   like:

   Signed-off-by: ...
   ---
   V3:
   * Added this, removed that
   
   v2:
   * Rewrite from scratch

- Keep all version of your patches in same email thread, this can be done
   via "git sent-email", '--in-reply-to' argument, please check documents.

   This helps to keep all version and change requests and changes grouped,
   so makes it easy to manage. Also for archives, it makes easier to find
   out all versions and see the stages of improvement, otherwise it is very
   hard to trace a past feature from archives.



And as far as I got, last patch you send makes the API experimental, but
the API is for drivers (not applications), so it should be defined in
'ethdev_driver.h' and marked as internal, also in INTERNAL group.
(I already commented this one of the other version of the patch)


Another thing is, there are some checkpatch warnings, you can see check
results in the "Checks" box of:
https://patches.dpdk.org/project/dpdk/patch/20220121042944.23929-1-kumaraparamesh92@gmail.com/
The "ci/checkpatch" test is colored as yellow, if you click it you will
see the warning.

And I am pretty sure internal tool './devtools/check-git-log.sh' will
have a few things to complain.
Can you please solve above checkpatch and check-git-log warnings in next
version?

Thanks,
ferruh