[ovs-dev] [PATCH] datapath: Fix MPLS action validation.

Andy Zhou azhou at nicira.com
Fri Dec 19 20:15:43 UTC 2014


It may be nice to check for inner protocol being set when executing
the set tunnel action.

On Fri, Dec 19, 2014 at 11:10 AM, Jarno Rajahalme <jrajahalme at nicira.com> wrote:
> LGTM,
>
> Acked-by: Jarno Rajahalme <jrajahalme at nicira.com>
>
>   Jarno
>
>> On Dec 19, 2014, at 10:50 AM, Pravin B Shelar <pshelar at nicira.com> wrote:
>>
>> Linux stack do not allow GSO for packet with multiple
>> encapsulations.  Therefore there was check in MPLS action
>> validation to detect such case, But is it not really required
>> since we already have check in action execution.
>> Removing this check also fixes bug in action copy to no skip
>> multiple set actions.
>>
>> Signed-off-by: Pravin B Shelar <pshelar at nicira.com>
>> Reported-by: Srinivas Neginhal <sneginha at vmware.com>
>> Bug #1367702
>> ---
>> datapath/flow_netlink.c |   13 ++-----------
>> 1 files changed, 2 insertions(+), 11 deletions(-)
>>
>> diff --git a/datapath/flow_netlink.c b/datapath/flow_netlink.c
>> index 4aae305..c611e71 100644
>> --- a/datapath/flow_netlink.c
>> +++ b/datapath/flow_netlink.c
>> @@ -1764,7 +1764,6 @@ static int __ovs_nla_copy_actions(const struct nlattr *attr,
>>                  __be16 eth_type, __be16 vlan_tci, bool log)
>> {
>>    const struct nlattr *a;
>> -    bool out_tnl_port = false;
>>    int rem, err;
>>
>>    if (depth >= SAMPLE_ACTION_DEPTH)
>> @@ -1807,7 +1806,6 @@ static int __ovs_nla_copy_actions(const struct nlattr *attr,
>>        case OVS_ACTION_ATTR_OUTPUT:
>>            if (nla_get_u32(a) >= DP_MAX_PORTS)
>>                return -EINVAL;
>> -            out_tnl_port = false;
>>
>>            break;
>>
>> @@ -1843,14 +1841,9 @@ static int __ovs_nla_copy_actions(const struct nlattr *attr,
>>        case OVS_ACTION_ATTR_PUSH_MPLS: {
>>            const struct ovs_action_push_mpls *mpls = nla_data(a);
>>
>> -            /* Networking stack do not allow simultaneous Tunnel
>> -             * and MPLS GSO.
>> -             */
>> -            if (out_tnl_port)
>> -                return -EINVAL;
>> -
>>            if (!eth_p_mpls(mpls->mpls_ethertype))
>>                return -EINVAL;
>> +
>>            /* Prohibit push MPLS other than to a white list
>>             * for packets that have a known tag order.
>>             */
>> @@ -1884,11 +1877,9 @@ static int __ovs_nla_copy_actions(const struct nlattr *attr,
>>
>>        case OVS_ACTION_ATTR_SET:
>>            err = validate_set(a, key, sfa,
>> -                       &out_tnl_port, eth_type, log);
>> +                       &skip_copy, eth_type, log);
>>            if (err)
>>                return err;
>> -
>> -            skip_copy = out_tnl_port;
>>            break;
>>
>>        case OVS_ACTION_ATTR_SAMPLE:
>> --
>> 1.7.1
>>
>> _______________________________________________
>> dev mailing list
>> dev at openvswitch.org
>> http://openvswitch.org/mailman/listinfo/dev
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev



More information about the dev mailing list