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 <[email protected]>

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
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to