netdev - Re: [PATCH net-next] openvswitch: kernel datapath clone action

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] [day] [month] [year] [list]

Date:   Fri, 29 Jun 2018 15:08:11 -0700
From:   Pravin Shelar <pshelar@....org>
To:     Yifeng Sun <pkusunyifeng@...il.com>
Cc:     Andy Zhou <azhou@....org>,
        Linux Kernel Network Developers <netdev@...r.kernel.org>
Subject: Re: [PATCH net-next] openvswitch: kernel datapath clone action

On Thu, Jun 28, 2018 at 8:20 AM, Yifeng Sun <pkusunyifeng@...il.com> wrote:
> Add 'clone' action to kernel datapath by using existing functions.
> When actions within clone don't modify the current flow, the flow
> key is not cloned before executing clone actions.
>
> This is a follow up patch for this incomplete work:
> https://patchwork.ozlabs.org/patch/722096/
>
> Signed-off-by: Yifeng Sun <pkusunyifeng@...il.com>
> Signed-off-by: Andy Zhou <azhou@....org>
> ---
>  include/uapi/linux/openvswitch.h |  8 +++++
>  net/openvswitch/actions.c        | 33 ++++++++++++++++++
>  net/openvswitch/flow_netlink.c   | 73 ++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 114 insertions(+)
>
> diff --git a/include/uapi/linux/openvswitch.h b/include/uapi/linux/openvswitch.h
> index 863aaba..5de8583 100644
> --- a/include/uapi/linux/openvswitch.h
> +++ b/include/uapi/linux/openvswitch.h
> @@ -625,6 +625,11 @@ struct sample_arg {
>                                       * 'OVS_SAMPLE_ATTR_PROBABILITY'.
>                                       */
>  };
> +
> +#define OVS_CLONE_ATTR_EXEC      0   /* Specify an u32 value. When nonzero,
> +                                     * actions in clone will not change flow
> +                                     * keys. False otherwise.
> +                                     */
>  #endif

This symbol is used only in datapath, so we can move it to kernel
headers from uapi.

> diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
> index 30a5df2..4444e31 100644
> --- a/net/openvswitch/actions.c
> +++ b/net/openvswitch/actions.c
> @@ -1057,6 +1057,28 @@ static int sample(struct datapath *dp, struct sk_buff *skb,
>                              clone_flow_key);
>  }
>
> +/* When 'last' is true, clone() should always consume the 'skb'.
> + * Otherwise, clone() should keep 'skb' intact regardless what
> + * actions are executed within clone().
> + */
> +static int clone(struct datapath *dp, struct sk_buff *skb,
> +                struct sw_flow_key *key, const struct nlattr *attr,
> +                bool last)
> +{
> +       struct nlattr *actions;
> +       struct nlattr *clone_arg;
> +       int rem = nla_len(attr);
> +       bool clone_flow_key;
> +
> +       /* The first action is always 'OVS_CLONE_ATTR_ARG'. */
> +       clone_arg = nla_data(attr);
> +       clone_flow_key = !nla_get_u32(clone_arg);
> +       actions = nla_next(clone_arg, &rem);
> +

Since OVS_CLONE_ATTR_EXEC means do not clone the key, it can be named
accordingly.