Re: [dpdk-dev] [PATCH] pdump: change to use generic multi-process channel

["Tan, Jianfeng" <jianfeng.tan@intel.com>]

> -----Original Message-----
> From: Pattan, Reshma
> Sent: Wednesday, March 21, 2018 5:55 PM
> To: Tan, Jianfeng; dev@dpdk.org
> Subject: RE: [PATCH] pdump: change to use generic multi-process channel
> 
> Hi,
> 
> > -----Original Message-----
> > From: Tan, Jianfeng
> > Sent: Wednesday, March 21, 2018 2:28 AM
> > To: Pattan, Reshma <reshma.pattan@intel.com>; dev@dpdk.org
> > Subject: RE: [PATCH] pdump: change to use generic multi-process channel
> >
> 
> Hi,
> 
> > > 1) I feel ABI breakage has to be addressed  first for change in
> > rte_pdump_init() .
> > > 2)ABI notice for removal of the rte_pdump_set_socket_dir()  and then
> > remove it completely .
> >
> > This patch itself does not break any ABI. It just puts parameters of
> > rte_pdump_init() not used. And make rte_pdump_set_socket_dir() as a
> > dummy function.
> >
> 
> So, for current release you just mark parameters unused and functions set to
> dummy, in future release you announce
> ABI breakage by removing them completely? If that is agreed plan I don't
> have any issues.

Actually, as you commented, we can announce the deprecation with this patch.

> 
> > > 3)Need to do cleanup of the code app/dpdk-pdump.
> >
> > Yes, I understand it's a normal process to announce deprecation firstly, and
> > then do the change.
> >
> > But here is the thing, with generic mp introduced, we will not be
> compatible
> > with DPDK versions.
> > So we want to unify the use of generic mp channel in this release for vfio,
> > pdump, vdev, memory rework.
> > And in fact, ABI/API changes could be delayed to later releases.
> 
> So, you want to remove unnecessary socket  related code from dpdk-pdump
> in future release itself?  Kind of making sense.
> But dpdk-pdump  tool has socket path related command line options which
> user still can pass on, isn't it kind of confusion we creating w.r.t
> Internal design and usage?

AFAIK, these options do not affect anything with this patch even they are set. How about printing a warning saying that these options will be deprecated and take no effect now?

I'll send a v2 for your review.

Thanks,
Jianfeng

> 
> >
> > > 4)After all the changes we need to make sure dpdk-pdump works fine
> > > without breaking the functionality, validation team should be able to help.
> >
> > I have done a simple test of pdump. Can you suggest where can I get the
> > comprehensive test cases?
> >
> 
> Ok, if you have verified and observed packets are been captured successfully,
> that is good enough.
> 
> Thanks,
> Reshma