[ovs-dev] [RFC v3 4/4] netdev-offload: Disallow offloading to unrelated tunneling vports.

[Ophir Munk]

> -----Original Message-----
> From: Ilya Maximets <i.maximets at ovn.org>
> Sent: Tuesday, December 3, 2019 1:13 PM
> To: ovs-dev at openvswitch.org
> Cc: Ophir Munk <ophirmu at mellanox.com>; Roni Bar Yanai
> <roniba at mellanox.com>; Simon Horman
> <simon.horman at netronome.com>; Ilya Maximets <i.maximets at ovn.org>
> Subject: [RFC v3 4/4] netdev-offload: Disallow offloading to unrelated
> tunneling vports.
> 
> 'linux_tc' flow API suitable only for tunneling vports with backing linux
> interfaces. DPDK flow API is not suitable for such ports.
> 
> With this change we could drop vport restriction from dpif-netdev.
> 
> This is a prerequisite for enabling vport offloading in DPDK.
> 
> Signed-off-by: Ilya Maximets <i.maximets at ovn.org>
> ---
>  lib/dpif-netdev.c         | 3 +--
>  lib/netdev-offload-dpdk.c | 8 ++++++++
>  lib/netdev-offload-tc.c   | 8 ++++++++
>  3 files changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index a78fd5909..945051e3b
> 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -2413,8 +2413,7 @@ dp_netdev_flow_offload_put(struct
> dp_flow_offload_item *offload)
>      info.flow_mark = mark;
> 
>      port = netdev_ports_get(in_port, dpif_type_str);
> -    if (!port || netdev_vport_is_vport_class(port->netdev_class)) {
> -        netdev_close(port);
> +    if (!port) {
>          goto err_free;
>      }
>      ret = netdev_flow_put(port, &offload->match, diff --git a/lib/netdev-
> offload-dpdk.c b/lib/netdev-offload-dpdk.c index 96794dc4d..28049788b
> 100644
> --- a/lib/netdev-offload-dpdk.c
> +++ b/lib/netdev-offload-dpdk.c
> @@ -22,6 +22,7 @@
>  #include "dpif-netdev.h"
>  #include "netdev-offload-provider.h"
>  #include "netdev-provider.h"
> +#include "netdev-vport.h"
>  #include "openvswitch/match.h"
>  #include "openvswitch/vlog.h"
>  #include "packets.h"
> @@ -758,6 +759,13 @@ netdev_offload_dpdk_flow_del(struct netdev
> *netdev, const ovs_u128 *ufid,  static int
> netdev_offload_dpdk_init_flow_api(struct netdev *netdev)  {
> +    if (netdev_vport_is_vport_class(netdev->netdev_class)
> +        && !strcmp(netdev_get_dpif_type(netdev), "system")) {
> +        VLOG_DBG("%s: vport belongs to the system datapath. Skipping.",
> +                 netdev_get_name(netdev));
> +        return EOPNOTSUPP;
> +    }
> +

It would be more consistent to reject any dpif which is not netdev such as:

   if (netdev_vport_is_vport_class(netdev->netdev_class)
       && strcmp(netdev_get_dpif_type(netdev), "netdev")) {
       VLOG_DBG("%s: vport doesn't belong to the netdev datapath. Skipping.",
                netdev_get_name(netdev));
       return EOPNOTSUPP;
   }

>      return netdev_dpdk_flow_api_supported(netdev) ? 0 : EOPNOTSUPP;  }
> 
> diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c index
> dee82aa9a..5501ad810 100644
> --- a/lib/netdev-offload-tc.c
> +++ b/lib/netdev-offload-tc.c
> @@ -31,6 +31,7 @@
>  #include "netdev-linux.h"
>  #include "netdev-offload-provider.h"
>  #include "netdev-provider.h"
> +#include "netdev-vport.h"
>  #include "netlink.h"
>  #include "netlink-socket.h"
>  #include "odp-netlink.h"
> @@ -1643,6 +1644,13 @@ netdev_tc_init_flow_api(struct netdev *netdev)
>      int ifindex;
>      int error;
> 
> +    if (netdev_vport_is_vport_class(netdev->netdev_class)
> +        && strcmp(netdev_get_dpif_type(netdev), "system")) {
> +        VLOG_DBG("%s: vport doesn't belong to the system datapath.
> Skipping.",
> +                 netdev_get_name(netdev));
> +        return EOPNOTSUPP;
> +    }
> +
>      ifindex = netdev_get_ifindex(netdev);
>      if (ifindex < 0) {
>          VLOG_INFO("init: failed to get ifindex for %s: %s",
> --
> 2.17.1