On 02.04.2019 12:18, Ophir Munk wrote:
> vport offloaded functions should have a different implementation for
> linux based OVS versus dpdk based OVS. Currently there is only support
> for linux based offloaded API. The code in the file named netdev-vport.c
> checks 'ifdef __linux__' to select the linux based offloaded functions,
> without checking 'ifndef DPDK_NETDEV' as well.
> '__linux__' is a pre-defined compiler macro, indicating that a source
> code is compiled on a linux based system. Any code inside a __linux__
> definition will be excluded on a windows based system, for example.
> 'DPDK_NETDEV' is a macro defined by autoconf tools when configuring OVS
> to be dpdk based as shown in [1].
> Before this commit and in case hw-offload=true - using a vport interface
> with a dpdk based OVS daemon running on a linux machine resulted in an
> error since the vport linux based offloaded APIs were called instead of
> returning EOPNOTSUPP. Luckily the linux offloaded API returned immediately
> on a get_ifindex() failure, which caused no harm. An example of the failure
> message is shown in [2].
>
> [1]
> configure --with-dpdk=<dpdk root tree>/<target architecture>
>
> [2]
> ovs|00002|netdev_tc_offloads(dp_netdev_flow_5)|ERR|flow_put: failed to
> get ifindex for vxlan_sys_4789: No such device
>
> Fixes: 01b257860c89 ("netdev-vport: Use common offloads interface")
> Signed-off-by: Ophir Munk <[email protected]>
> ---
Hi.
We already discussed this with Roni some time ago. You can't just disable
vport offloading on compile time. Compiling with DPDK support doesn't mean
that it will be used only with userspace datapath. DPDK support is just an
additional feature. You could still use kernel datapath and even use kernel
and userpace datapaths at the same time under control of the same ovs-vswitchd
process. If those error messages are annoying, you need to *check* if offloading
supported by *each particular netdev-vport in runtime*, probably based on the
datapath type they assigned to. In case of simultaneous running of different
datapaths some vports will have backing linux devices and some will not.
Compiling out offloading based on enabling of DPDK support will just break
offloading for kernel datapath. At least, this will cause issues for distros
that will have to decide if they want DPDK support or vport offloading in
kernel.
Best regards, Ilya Maximets.
> lib/netdev-vport.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c
> index 808a43f..5ba7455 100644
> --- a/lib/netdev-vport.c
> +++ b/lib/netdev-vport.c
> @@ -47,8 +47,8 @@
> #include "unaligned.h"
> #include "unixctl.h"
> #include "openvswitch/vlog.h"
> +#if defined(__linux__) && !defined(DPDK_NETDEV)
> #include "netdev-tc-offloads.h"
> -#ifdef __linux__
> #include "netdev-linux.h"
> #endif
>
> @@ -1093,7 +1093,7 @@ netdev_vport_get_pt_mode(const struct netdev *netdev)
>
>
>
> -#ifdef __linux__
> +#if defined(__linux__) && !defined(DPDK_NETDEV)
> static int
> netdev_vport_get_ifindex(const struct netdev *netdev_)
> {
> @@ -1105,10 +1105,10 @@ netdev_vport_get_ifindex(const struct netdev *netdev_)
>
> #define NETDEV_VPORT_GET_IFINDEX netdev_vport_get_ifindex
> #define NETDEV_FLOW_OFFLOAD_API , LINUX_FLOW_OFFLOAD_API
> -#else /* !__linux__ */
> +#else
> #define NETDEV_VPORT_GET_IFINDEX NULL
> #define NETDEV_FLOW_OFFLOAD_API
> -#endif /* __linux__ */
> +#endif
>
> #define VPORT_FUNCTIONS_COMMON \
> .run = netdev_vport_run, \
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
