On Wed, Aug 31, 2016 at 9:39 PM, Eric Dumazet <[email protected]> wrote:
> On Wed, Aug 31, 2016 at 5:46 AM, Hadar Hen Zion <[email protected]> wrote:
>>
>> From: Amir Vadai <[email protected]>
>>
>> This action could be used before redirecting packets to a shared tunnel
>> device, or when redirecting packets arriving from a such a device.
>>
>>
>> +
>> +struct tcf_tunnel_key_params {
>> + struct rcu_head rcu;
>> + int tcft_action;
>
> Also add " int action;"
>
> (see why later)
>
>> + struct metadata_dst *tcft_enc_metadata;
>> +};
>> +
>
>
>
>> +
>> +static int tunnel_key_act(struct sk_buff *skb, const struct tc_action *a,
>> + struct tcf_result *res)
>> +{
>> + struct tcf_tunnel_key *t = to_tunnel_key(a);
>> + struct tcf_tunnel_key_params *params;
>> + int action;
>> +
>> + rcu_read_lock();
>> +
>> + params = rcu_dereference(t->params);
>> +
>> + tcf_lastuse_update(&t->tcf_tm);
>> + bstats_cpu_update(this_cpu_ptr(t->common.cpu_bstats), skb);
>> + action = t->tcf_action;
>
> Ideally, you should read param->action instead of t->tcf_action to be
> completely clean.
As you suggested above, I can do it by adding "int action" to struct
tcf_tunnel_key_paramse.
But, it means that act_tunnel_key would have a different behavior than
all the other actions and even though
"struct tc_action" has a designated parameters to store this action we
won't use it.
So it won't be completely clean...
Do you think we have a cleaner way to protect it?
>
>> +
>> + switch (params->tcft_action) {
>> + case TCA_TUNNEL_KEY_ACT_RELEASE:
>> + skb_dst_drop(skb);
>> + break;
>> + case TCA_TUNNEL_KEY_ACT_SET:
>> + skb_dst_drop(skb);
>> + skb_dst_set(skb, dst_clone(¶ms->tcft_enc_metadata->dst));
>> + break;
>> + default:
>> + WARN_ONCE(1, "Bad tunnel_key action.\n");
>> + break;
>> + }
>> +
>> + rcu_read_unlock();
>> +
>> + return action;
>> +}
>>
