netdev - Re: [PATCH net-next] net: openvswitch: Set OvS recirc

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 for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <5465b87e-9365-54d2-8b2f-f2107e33ad79@mellanox.com>
Date:   Mon, 19 Aug 2019 06:20:26 +0000
From:   Paul Blakey <paulb@...lanox.com>
To:     Pravin B Shelar <pshelar@....org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "David S. Miller" <davem@...emloft.net>,
        Justin Pettit <jpettit@...ira.com>,
        Simon Horman <simon.horman@...ronome.com>,
        Marcelo Ricardo Leitner <marcelo.leitner@...il.com>,
        Vlad Buslov <vladbu@...lanox.com>
CC:     Jiri Pirko <jiri@...lanox.com>, Roi Dayan <roid@...lanox.com>,
        Yossi Kuperman <yossiku@...lanox.com>,
        Rony Efraim <ronye@...lanox.com>, Oz Shlomo <ozsh@...lanox.com>
Subject: Re: [PATCH net-next] net: openvswitch: Set OvS recirc_id from tc
 chain


On 8/18/2019 7:00 PM, Paul Blakey wrote:
> What do you guys say about the following diff on top of the last one?
> Use static key, and also have OVS_DP_CMD_SET command probe/enable the feature.
>
> This will allow userspace to probe the feature, and selectivly enable it via the
> OVS_DP_CMD_SET command.
>
> Thansk,
> Paul.
>
>
> ---
>   include/uapi/linux/openvswitch.h |  3 +++
>   net/openvswitch/datapath.c       | 29 +++++++++++++++++++++++++----
>   net/openvswitch/datapath.h       |  2 ++
>   net/openvswitch/flow.c           |  6 ++++--
>   4 files changed, 34 insertions(+), 6 deletions(-)
>
> diff --git a/include/uapi/linux/openvswitch.h b/include/uapi/linux/openvswitch.h
> index f271f1e..1887a45 100644
> --- a/include/uapi/linux/openvswitch.h
> +++ b/include/uapi/linux/openvswitch.h
> @@ -123,6 +123,9 @@ struct ovs_vport_stats {
>   /* Allow datapath to associate multiple Netlink PIDs to each vport */
>   #define OVS_DP_F_VPORT_PIDS	(1 << 1)
>   
> +/* Allow tc offload recirc sharing */
> +#define OVS_DP_F_TC_RECIRC_SHARING	(1 << 2)
> +
>   /* Fixed logical ports. */
>   #define OVSP_LOCAL      ((__u32)0)
>   
> diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
> index 892287d..589b4f1 100644
> --- a/net/openvswitch/datapath.c
> +++ b/net/openvswitch/datapath.c
> @@ -1541,10 +1541,27 @@ static void ovs_dp_reset_user_features(struct sk_buff *skb, struct genl_info *in
>   	dp->user_features = 0;
>   }
>   
> -static void ovs_dp_change(struct datapath *dp, struct nlattr *a[])
> +DEFINE_STATIC_KEY_FALSE(tc_recirc_sharing_support);
> +
> +static int ovs_dp_change(struct datapath *dp, struct nlattr *a[])
>   {
> +	u32 user_features;
> +
>   	if (a[OVS_DP_ATTR_USER_FEATURES])
> -		dp->user_features = nla_get_u32(a[OVS_DP_ATTR_USER_FEATURES]);
> +		user_features = nla_get_u32(a[OVS_DP_ATTR_USER_FEATURES]);
> +
> +#if !IS_ENABLED(CONFIG_NET_TC_SKB_EXT)
> +	if (user_features & OVS_DP_F_TC_RECIRC_SHARING)
> +		return -EOPNOTSUPP;
> +#endif
> +	dp->user_features = user_features;
> +
> +	if (dp->user_features & OVS_DP_F_TC_RECIRC_SHARING)
> +		static_branch_enable(&tc_recirc_sharing_support);
> +	else
> +		static_branch_disable(&tc_recirc_sharing_support);
> +
> +	return 0;
>   }
>   
>   static int ovs_dp_cmd_new(struct sk_buff *skb, struct genl_info *info)
> @@ -1606,7 +1623,9 @@ static int ovs_dp_cmd_new(struct sk_buff *skb, struct genl_info *info)
>   	parms.port_no = OVSP_LOCAL;
>   	parms.upcall_portids = a[OVS_DP_ATTR_UPCALL_PID];
>   
> -	ovs_dp_change(dp, a);
> +	err = ovs_dp_change(dp, a);
> +	if (err)
> +		goto err_destroy_meters;
>   
>   	/* So far only local changes have been made, now need the lock. */
>   	ovs_lock();
> @@ -1732,7 +1751,9 @@ static int ovs_dp_cmd_set(struct sk_buff *skb, struct genl_info *info)
>   	if (IS_ERR(dp))
>   		goto err_unlock_free;
>   
> -	ovs_dp_change(dp, info->attrs);
> +	err = ovs_dp_change(dp, info->attrs);
> +	if (err)
> +		goto err_unlock_free;
>   
>   	err = ovs_dp_cmd_fill_info(dp, reply, info->snd_portid,
>   				   info->snd_seq, 0, OVS_DP_CMD_SET);
> diff --git a/net/openvswitch/datapath.h b/net/openvswitch/datapath.h
> index 751d34a..81e85dd 100644
> --- a/net/openvswitch/datapath.h
> +++ b/net/openvswitch/datapath.h
> @@ -218,6 +218,8 @@ static inline struct datapath *get_dp(struct net *net, int dp_ifindex)
>   extern struct notifier_block ovs_dp_device_notifier;
>   extern struct genl_family dp_vport_genl_family;
>   
> +DECLARE_STATIC_KEY_FALSE(tc_recirc_sharing_support);
> +
>   void ovs_dp_process_packet(struct sk_buff *skb, struct sw_flow_key *key);
>   void ovs_dp_detach_port(struct vport *);
>   int ovs_dp_upcall(struct datapath *, struct sk_buff *,
> diff --git a/net/openvswitch/flow.c b/net/openvswitch/flow.c
> index 0287ead..c0ac7c9 100644
> --- a/net/openvswitch/flow.c
> +++ b/net/openvswitch/flow.c
> @@ -853,8 +853,10 @@ int ovs_flow_key_extract(const struct ip_tunnel_info *tun_info,
>   	key->mac_proto = res;
>   
>   #if IS_ENABLED(CONFIG_NET_TC_SKB_EXT)
> -	tc_ext = skb_ext_find(skb, TC_SKB_EXT);
> -	key->recirc_id = tc_ext ? tc_ext->chain : 0;
> +	if (static_branch_unlikely(&tc_recirc_sharing_support)) {
> +		tc_ext = skb_ext_find(skb, TC_SKB_EXT);
> +		key->recirc_id = tc_ext ? tc_ext->chain : 0;
> +	}

Here should be

else

       key->recirc_id = 0

>   #else
>   	key->recirc_id = 0;
>   #endif