Hi Ilya,
Please see comments inline
> -----Original Message-----
> From: Ilya Maximets <[email protected]>
...
> 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,
The main issue I am concerned with is that unexpected/unplanned kernel targeted
code is executed
as part of dpdk run. It may now end with annoying messages, but I think kernel
code should be avoided
in the first place in case of dpdk datapath. I will send a new patch to address
it.
> you need to
> *check* if offloading supported by *each particular netdev-vport in
> runtime*, probably based on the datapath type they assigned to.
Thank you for this advice. Do you have pointers into the code where such checks
occur?
> 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
