[ovs-dev] [PATCH 3/3] ofproto-dpif: Validate ct action support.

Joe Stringer joestringer at nicira.com
Tue Nov 10 21:56:51 UTC 2015


On 9 November 2015 at 17:25, Jarno Rajahalme <jarno at ovn.org> wrote:
>
>> On Nov 7, 2015, at 12:05 PM, Joe Stringer <joestringer at nicira.com> wrote:
>>
>> Disallow installing rules that execute ct() if conntrack is unsupported
>> in the datapath.
>>
>> Reported-by: Ravindra Kenchappa <ravindra.kenchappa at hpe.com>
>> Signed-off-by: Joe Stringer <joestringer at nicira.com>
>> ---
>> ofproto/ofproto-dpif.c | 44 +++++++++++++++++++++++++++++++++++++++++++-
>> 1 file changed, 43 insertions(+), 1 deletion(-)
>>
>> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
>> index 2f75b93d9694..e09c28bb15ed 100644
>> --- a/ofproto/ofproto-dpif.c
>> +++ b/ofproto/ofproto-dpif.c
>> @@ -4048,6 +4048,44 @@ check_flow(const struct ofproto_dpif *ofproto, const struct miniflow *flow,
>> }
>>
>> static enum ofperr
>> +check_actions(const struct ofproto_dpif *ofproto,
>> +              const struct rule_actions *const actions)
>> +{
>> +    const struct ofpact *ofpact;
>> +
>> +    OFPACT_FOR_EACH (ofpact, actions->ofpacts, actions->ofpacts_len) {
>> +        const struct odp_support *support;
>> +        const struct ofpact_conntrack *ct;
>> +        const struct ofpact *a;
>> +
>> +        if (ofpact->type != OFPACT_CT) {
>> +            continue;
>> +        }
>> +
>> +        ct = CONTAINER_OF(ofpact, struct ofpact_conntrack, ofpact);
>> +        support = &ofproto_dpif_get_support(ofproto)->odp;
>> +
>> +        if (!support->ct_state) {
>> +            return OFPERR_OFPBAC_BAD_TYPE;
>> +        }
>> +        if ((ct->zone_imm || ct->zone_src.field) && !support->ct_zone) {
>> +            return OFPERR_OFPBAC_BAD_ARGUMENT;
>> +        }
>> +
>> +        OFPACT_FOR_EACH(a, ct->actions, ofpact_ct_get_action_len(ct)) {
>> +            const struct mf_field *dst = ofpact_get_mf_dst(a);
>> +
>> +            if (dst && ((dst->id == MFF_CT_MARK && !support->ct_mark)
>> +                        || (dst->id == MFF_CT_LABEL && !support->ct_label))) {
>> +                return OFPERR_OFPBAC_BAD_SET_ARGUMENT;
>> +            }
>> +        }
>> +    }
>> +
>> +    return 0;
>> +}
>
>
> We already loop through the actions for a similar purpose in ofproto_check_ofpacts(). Maybe make something like is_action_supported() accessible via the ofproto class and call that for the conntrack action from ofproto_check_ofpacts()?

Makes sense, I'll rework this patch do to it like this.



More information about the dev mailing list