| Message ID | 20191101120633.28445-1-i.maximets@ovn.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [ovs-dev,RFC] dpdk: Deprecate pdump support. | expand |
Ilya Maximets <i.maximets@ovn.org> writes: > 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. > > Claiming to completely remove this functionality from OVS in one > of the next releases. > > Signed-off-by: Ilya Maximets <i.maximets@ovn.org> > --- Acked-by: Aaron Conole <aconole@redhat.com>
Hi Ilya, On Fri, 1 Nov 2019 13:06:33 +0100 Ilya Maximets <i.maximets@ovn.org> 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? fbl
On 04.11.2019 17:21, Flavio Leitner wrote: > > Hi Ilya, > > On Fri, 1 Nov 2019 13:06:33 +0100 > Ilya Maximets <i.maximets@ovn.org> 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? Best regards, Ilya Maximets.
On Mon, 4 Nov 2019 18:16:26 +0100 Ilya Maximets <i.maximets@ovn.org> wrote: > On 04.11.2019 17:21, Flavio Leitner wrote: > > > > Hi Ilya, > > > > On Fri, 1 Nov 2019 13:06:33 +0100 > > Ilya Maximets <i.maximets@ovn.org> 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
diff --git a/.travis/linux-build.sh b/.travis/linux-build.sh index 69260181b..4e74973a3 100755 --- a/.travis/linux-build.sh +++ b/.travis/linux-build.sh @@ -124,7 +124,7 @@ function install_dpdk() sed -i '/CONFIG_RTE_EAL_IGB_UIO=y/s/=y/=n/' build/.config sed -i '/CONFIG_RTE_KNI_KMOD=y/s/=y/=n/' build/.config - # Enable pdump. This will enable building of the relevant OVS code. + # Enable pdump support in DPDK. sed -i '/CONFIG_RTE_LIBRTE_PMD_PCAP=n/s/=n/=y/' build/.config sed -i '/CONFIG_RTE_LIBRTE_PDUMP=n/s/=n/=y/' build/.config @@ -168,6 +168,8 @@ if [ "$DPDK" ] || [ "$DPDK_SHARED" ]; then DPDK_VER="18.11.2" fi install_dpdk $DPDK_VER + # Enable pdump support in OVS. + EXTRA_OPTS="${EXTRA_OPTS} --enable-dpdk-pdump" if [ "$CC" = "clang" ]; then # Disregard cast alignment errors until DPDK is fixed CFLAGS_FOR_OVS="${CFLAGS_FOR_OVS} -Wno-cast-align" diff --git a/Documentation/topics/dpdk/pdump.rst b/Documentation/topics/dpdk/pdump.rst index 7bd1d3e9f..b4d8aa8e9 100644 --- a/Documentation/topics/dpdk/pdump.rst +++ b/Documentation/topics/dpdk/pdump.rst @@ -27,10 +27,16 @@ pdump .. versionadded:: 2.6.0 +.. warning:: + + DPDK pdump support is deprecated in OVS and will be removed in next + releases. + pdump allows you to listen on DPDK ports and view the traffic that is passing on them. To use this utility, one must have libpcap installed on the system. Furthermore, DPDK must be built with ``CONFIG_RTE_LIBRTE_PDUMP=y`` and -``CONFIG_RTE_LIBRTE_PMD_PCAP=y``. +``CONFIG_RTE_LIBRTE_PMD_PCAP=y``. OVS should be built with +``--enable-dpdk-pdump`` configuration option. .. warning:: diff --git a/NEWS b/NEWS index 88b818948..0d65d5a7f 100644 --- a/NEWS +++ b/NEWS @@ -10,6 +10,10 @@ Post-v2.12.0 if supported by libbpf. * Add option to enable, disable and query TCP sequence checking in conntrack. + - DPDK: + * DPDK pdump packet capture support disabled by default. New configure + option '--enable-dpdk-pdump' to enable it. + * DPDK pdump support is deprecated and will be removed in next releases. v2.12.0 - 03 Sep 2019 --------------------- diff --git a/acinclude.m4 b/acinclude.m4 index a0507cfe0..588a0bdf2 100644 --- a/acinclude.m4 +++ b/acinclude.m4 @@ -343,12 +343,24 @@ AC_DEFUN([OVS_CHECK_DPDK], [ AC_DEFINE([VHOST_NUMA], [1], [NUMA Aware vHost support detected in DPDK.]) ], [], [[#include <rte_config.h>]]) - AC_CHECK_DECL([RTE_LIBRTE_PMD_PCAP], [ - OVS_FIND_DEPENDENCY([pcap_dump], [pcap], [libpcap]) - AC_CHECK_DECL([RTE_LIBRTE_PDUMP], [ - AC_DEFINE([DPDK_PDUMP], [1], [DPDK pdump enabled in OVS.]) - ], [], [[#include <rte_config.h>]]) - ], [], [[#include <rte_config.h>]]) + AC_MSG_CHECKING([whether DPDK pdump support is enabled]) + AC_ARG_ENABLE( + [dpdk-pdump], + [AC_HELP_STRING([--enable-dpdk-pdump], + [Enable DPDK pdump packet capture support])], + [AC_MSG_RESULT([yes]) + AC_MSG_WARN([DPDK pdump is deprecated, consider using ovs-tcpdump instead]) + AC_CHECK_DECL([RTE_LIBRTE_PMD_PCAP], [ + OVS_FIND_DEPENDENCY([pcap_dump], [pcap], [libpcap]) + AC_CHECK_DECL([RTE_LIBRTE_PDUMP], [ + AC_DEFINE([DPDK_PDUMP], [1], [DPDK pdump enabled in OVS.]) + ], [ + AC_MSG_ERROR([RTE_LIBRTE_PDUMP is not defined in rte_config.h]) + ], [[#include <rte_config.h>]]) + ], [ + AC_MSG_ERROR([RTE_LIBRTE_PMD_PCAP is not defined in rte_config.h]) + ], [[#include <rte_config.h>]])], + [AC_MSG_RESULT([no])]) AC_CHECK_DECL([RTE_LIBRTE_MLX5_PMD], [dnl found OVS_FIND_DEPENDENCY([mnl_attr_put], [mnl], [libmnl]) diff --git a/lib/dpdk.c b/lib/dpdk.c index f90cda75a..21dd47e80 100644 --- a/lib/dpdk.c +++ b/lib/dpdk.c @@ -434,6 +434,8 @@ dpdk_init__(const struct smap *ovs_other_config) #ifdef DPDK_PDUMP VLOG_INFO("DPDK pdump packet capture enabled"); + VLOG_WARN("DPDK pdump support is deprecated and " + "will be removed in next OVS releases."); err = rte_pdump_init(ovs_rundir()); if (err) { VLOG_INFO("Error initialising DPDK pdump");
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. Claiming to completely remove this functionality from OVS in one of the next releases. Signed-off-by: Ilya Maximets <i.maximets@ovn.org> --- .travis/linux-build.sh | 4 +++- Documentation/topics/dpdk/pdump.rst | 8 +++++++- NEWS | 4 ++++ acinclude.m4 | 24 ++++++++++++++++++------ lib/dpdk.c | 2 ++ 5 files changed, 34 insertions(+), 8 deletions(-)