netdev - Re: [patch net-next 10/13] openvswitch: add support for datapath hardware offload

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <540743B4.9080500@gmail.com>
Date:	Wed, 03 Sep 2014 09:37:08 -0700
From:	John Fastabend <john.fastabend@...il.com>
To:	Jiri Pirko <jiri@...nulli.us>
CC:	netdev@...r.kernel.org, davem@...emloft.net, nhorman@...driver.com,
	andy@...yhouse.net, tgraf@...g.ch, dborkman@...hat.com,
	ogerlitz@...lanox.com, jesse@...ira.com, pshelar@...ira.com,
	azhou@...ira.com, ben@...adent.org.uk, stephen@...workplumber.org,
	jeffrey.t.kirsher@...el.com, vyasevic@...hat.com,
	xiyou.wangcong@...il.com, john.r.fastabend@...el.com,
	edumazet@...gle.com, jhs@...atatu.com, sfeldma@...ulusnetworks.com,
	f.fainelli@...il.com, roopa@...ulusnetworks.com,
	linville@...driver.com, dev@...nvswitch.org, jasowang@...hat.com,
	ebiederm@...ssion.com, nicolas.dichtel@...nd.com,
	ryazanov.s.a@...il.com, buytenh@...tstofly.org,
	aviadr@...lanox.com, nbd@...nwrt.org, alexei.starovoitov@...il.com,
	Neil.Jerram@...aswitch.com, ronye@...lanox.com
Subject: Re: [patch net-next 10/13] openvswitch: add support for datapath
 hardware offload

On 09/03/2014 02:24 AM, Jiri Pirko wrote:
> Benefit from the possibility to work with flows in switch devices and
> use the swdev api to offload flow datapath.
>
> Signed-off-by: Jiri Pirko <jiri@...nulli.us>
> ---
>   net/openvswitch/Makefile       |   3 +-
>   net/openvswitch/datapath.c     |  33 ++++++
>   net/openvswitch/datapath.h     |   3 +
>   net/openvswitch/flow_table.c   |   1 +
>   net/openvswitch/hw_offload.c   | 245 +++++++++++++++++++++++++++++++++++++++++
>   net/openvswitch/hw_offload.h   |  22 ++++
>   net/openvswitch/vport-netdev.c |   3 +
>   net/openvswitch/vport.h        |   2 +
>   8 files changed, 311 insertions(+), 1 deletion(-)
>   create mode 100644 net/openvswitch/hw_offload.c
>   create mode 100644 net/openvswitch/hw_offload.h
>
> diff --git a/net/openvswitch/Makefile b/net/openvswitch/Makefile
> index 3591cb5..5152437 100644
> --- a/net/openvswitch/Makefile
> +++ b/net/openvswitch/Makefile
> @@ -13,7 +13,8 @@ openvswitch-y := \
>   	flow_table.o \
>   	vport.o \
>   	vport-internal_dev.o \
> -	vport-netdev.o
> +	vport-netdev.o \
> +	hw_offload.o
>
>   ifneq ($(CONFIG_OPENVSWITCH_VXLAN),)
>   openvswitch-y += vport-vxlan.o
> diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
> index 75bb07f..3e43e1d 100644
> --- a/net/openvswitch/datapath.c
> +++ b/net/openvswitch/datapath.c
> @@ -57,6 +57,7 @@
>   #include "flow_netlink.h"
>   #include "vport-internal_dev.h"
>   #include "vport-netdev.h"
> +#include "hw_offload.h"
>
>   int ovs_net_id __read_mostly;
>
> @@ -864,6 +865,9 @@ static int ovs_flow_cmd_new(struct sk_buff *skb, struct genl_info *info)
>   			acts = NULL;
>   			goto err_unlock_ovs;
>   		}
> +		error = ovs_hw_flow_insert(dp, new_flow);
> +		if (error)
> +			pr_warn("failed to insert flow into hw\n");

This is really close to silently failing. I think we need to
hard fail here somehow and push it back to userspace as part of
the reply and ovs_notify.

Otherwise I don't know how to manage the hardware correctly. Consider
the hardware table is full. In this case user space will continue to
add rules and they will be silently discarded. Similarly if user space
adds a flow/action that can not be supported by the hardware it will
be silently ignored.

Even if we do careful accounting on resources in user space we could
still get an ENOMEM error from sw_flow_action_create.

Same comment for the other hw commands flush/remove.

>   		if (unlikely(reply)) {
>   			error = ovs_flow_cmd_fill_info(new_flow,
> @@ -896,10 +900,18 @@ static int ovs_flow_cmd_new(struct sk_buff *skb, struct genl_info *info)
>   				goto err_unlock_ovs;
>   			}
>   		}


[...]


Thanks,
John

-- 
John Fastabend         Intel Corporation
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html