[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <B9AEC3C3-180D-45DB-84C2-B910C65FFA5C@redhat.com>
Date: Wed, 14 Dec 2022 16:20:21 +0100
From: Eelco Chaudron <echaudro@...hat.com>
To: netdev@...r.kernel.org
Cc: dev@...nvswitch.org, i.maximets@....org, edumazet@...gle.com,
kuba@...nel.org, pabeni@...hat.com, davem@...emloft.net
Subject: Re: [ovs-dev] [PATCH net] openvswitch: Fix flow lookup to use
unmasked key
On 14 Dec 2022, at 16:14, Eelco Chaudron wrote:
> The commit mentioned below causes the ovs_flow_tbl_lookup() function
> to be called with the masked key. However, it's supposed to be called
> with the unmasked key.
>
> This change reverses the commit below, but rather than having the key
> on the stack, it's allocated.
>
> Fixes: 190aa3e77880 ("openvswitch: Fix Frame-size larger than 1024 bytes warning.")
>
> Signed-off-by: Eelco Chaudron <echaudro@...hat.com>
Please ignore this version, as I sent out out without doing a stg refresh :(
> ---
> net/openvswitch/datapath.c | 25 ++++++++++++++++---------
> 1 file changed, 16 insertions(+), 9 deletions(-)
>
> diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
> index 861dfb8daf4a..23b233caa7fd 100644
> --- a/net/openvswitch/datapath.c
> +++ b/net/openvswitch/datapath.c
> @@ -948,6 +948,7 @@ static int ovs_flow_cmd_new(struct sk_buff *skb, struct genl_info *info)
> struct sw_flow_mask mask;
> struct sk_buff *reply;
> struct datapath *dp;
> + struct sw_flow_key *key;
> struct sw_flow_actions *acts;
> struct sw_flow_match match;
> u32 ufid_flags = ovs_nla_get_ufid_flags(a[OVS_FLOW_ATTR_UFID_FLAGS]);
> @@ -975,24 +976,26 @@ static int ovs_flow_cmd_new(struct sk_buff *skb, struct genl_info *info)
> }
>
> /* Extract key. */
> - ovs_match_init(&match, &new_flow->key, false, &mask);
> + key = kzalloc(sizeof(*key), GFP_KERNEL);
> + if (!key) {
> + error = -ENOMEN;
> + goto err_kfree_key;
> + }
> +
> + ovs_match_init(&match, key, false, &mask);
> error = ovs_nla_get_match(net, &match, a[OVS_FLOW_ATTR_KEY],
> a[OVS_FLOW_ATTR_MASK], log);
> if (error)
> goto err_kfree_flow;
>
> + ovs_flow_mask_key(&new_flow->key, key, true, &mask);
> +
> /* Extract flow identifier. */
> error = ovs_nla_get_identifier(&new_flow->id, a[OVS_FLOW_ATTR_UFID],
> - &new_flow->key, log);
> + key, log);
> if (error)
> goto err_kfree_flow;
>
> - /* unmasked key is needed to match when ufid is not used. */
> - if (ovs_identifier_is_key(&new_flow->id))
> - match.key = new_flow->id.unmasked_key;
> -
> - ovs_flow_mask_key(&new_flow->key, &new_flow->key, true, &mask);
> -
> /* Validate actions. */
> error = ovs_nla_copy_actions(net, a[OVS_FLOW_ATTR_ACTIONS],
> &new_flow->key, &acts, log);
> @@ -1019,7 +1022,7 @@ static int ovs_flow_cmd_new(struct sk_buff *skb, struct genl_info *info)
> if (ovs_identifier_is_ufid(&new_flow->id))
> flow = ovs_flow_tbl_lookup_ufid(&dp->table, &new_flow->id);
> if (!flow)
> - flow = ovs_flow_tbl_lookup(&dp->table, &new_flow->key);
> + flow = ovs_flow_tbl_lookup(&dp->table, key);
> if (likely(!flow)) {
> rcu_assign_pointer(new_flow->sf_acts, acts);
>
> @@ -1089,6 +1092,8 @@ static int ovs_flow_cmd_new(struct sk_buff *skb, struct genl_info *info)
>
> if (reply)
> ovs_notify(&dp_flow_genl_family, reply, info);
> +
> + kfree(key);
> return 0;
>
> err_unlock_ovs:
> @@ -1098,6 +1103,8 @@ static int ovs_flow_cmd_new(struct sk_buff *skb, struct genl_info *info)
> ovs_nla_free_flow_actions(acts);
> err_kfree_flow:
> ovs_flow_free(new_flow, false);
> +err_kfree_key:
> + kfree(key);
> error:
> return error;
> }
>
> _______________________________________________
> dev mailing list
> dev@...nvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Powered by blists - more mailing lists