On Mon, 4 Nov 2019 18:16:26 +0100
Ilya Maximets <[email protected]> wrote:

> On 04.11.2019 17:21, Flavio Leitner wrote:
> > 
> > Hi Ilya,
> > 
> > On Fri,  1 Nov 2019 13:06:33 +0100
> > Ilya Maximets <[email protected]> wrote:
> >   
> >> The conventional way for packet dumping in OVS is to use
> >> ovs-tcpdump that works via traffic mirroring.  DPDK pdump could
> >> probably be used for some lower level debugging, but it is not
> >> commonly used for various reasons.
> >>
> >> There are lots of limitations for using this functionality in
> >> practice. Most of them connected with running secondary pdump
> >> process and memory layout issues like requirement to disable ASLR
> >> in kernel. More details are available in DPDK guide:
> >> https://doc.dpdk.org/guides/prog_guide/multi_proc_support.html#multi-process-limitations
> >>
> >> Beside the functional limitations it's also hard to use this
> >> functionality correctly.  User must be sure that OVS and pdump
> >> utility are running on different CPU cores, which is hard because
> >> non-PMD threads could float over available CPU cores.  This or any
> >> other misconfiguration will likely lead to crash of the pdump
> >> utility or/and OVS.
> >>
> >> Another problem is that the user must actually have this special
> >> pdump utility in a system and it might be not available in
> >> distributions.
> >>
> >> This change disables pdump support by default introducing special
> >> configuration option '--enable-dpdk-pdump'.  Deprecation warnings
> >> will be shown to users on configuration and in runtime.  
> > 
> > The deprecation idea sounds good to me, but I don't see a reason to
> > break the existing behavior if we are going to remove that later on.
> > Wouldn't just the deprecation notice for next release be sufficient
> > and then we remove in the after release?  
> 
> I just thought that it might be unnecessary to put warnings in logs
> for users who doesn't care about pdump support.  Hiding the feature
> under configuration option I made deprecation warning visible to only
> those users who enabled it explicitly, i.e. wants to use it.
> Do you think it's better to print warnings for everyone?

Right, I missed that CONFIG_RTE_LIBRTE_PDUMP is enabled by default.
Well, users are supposed to read the NEWS file when rebasing, and you
already added a notice about the new option, so looks like your patch
is the way to go.

Thanks,
fbl


_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to