Thanks for the reviews, Jarno!
On 04/12/2015 15:03, "Jarno Rajahalme" <ja...@ovn.org> wrote:
>Thanks Daniele!
>
>Acked-by: Jarno Rajahalme <ja...@ovn.org>
>
>Pushed to master,
>
> Jarno
>
>> On Dec 3, 2015, at 10:35 AM, Daniele Di Proietto
>><diproiet...@vmware.com> wrote:
>>
>> The NAT validation is similar (and based on) the existing conntrack
>> validation: when a dpif backer is created, we try to install a flow with
>> the ct_state NAT bits set. If the flow setup fails we assume that the
>> backer doesn't support NAT and we reject OpenFlow flows with a NAT
>> action or a match on the ct_state NAT bits.
>>
>> Signed-off-by: Daniele Di Proietto <diproiet...@vmware.com>
>> ---
>> lib/odp-util.h | 5 +++++
>> ofproto/ofproto-dpif.c | 22 ++++++++++++++++------
>> 2 files changed, 21 insertions(+), 6 deletions(-)
>>
>> diff --git a/lib/odp-util.h b/lib/odp-util.h
>> index a936416..35849ae 100644
>> --- a/lib/odp-util.h
>> +++ b/lib/odp-util.h
>> @@ -178,6 +178,11 @@ struct odp_support {
>> bool ct_zone;
>> bool ct_mark;
>> bool ct_label;
>> +
>> + /* If true, it means that the datapath supports the NAT bits in
>> + * 'ct_state'. The above 'ct_state' member must be true for this
>> + * to make sense */
>> + bool ct_state_nat;
>> };
>>
>> struct odp_flow_key_parms {
>> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
>> index e76d8fe..d920ed0 100644
>> --- a/ofproto/ofproto-dpif.c
>> +++ b/ofproto/ofproto-dpif.c
>> @@ -1236,7 +1236,7 @@ check_masked_set_action(struct dpif_backer
>>*backer)
>> return !error;
>> }
>>
>> -#define CHECK_FEATURE__(NAME, FIELD)
>> \
>> +#define CHECK_FEATURE__(NAME, SUPPORT, FIELD, VALUE)
>> \
>> static bool
>> \
>> check_##NAME(struct dpif_backer *backer)
>> \
>> {
>> \
>> @@ -1247,12 +1247,12 @@ check_##NAME(struct dpif_backer *backer)
>> \
>> struct odp_flow_key_parms odp_parms = {
>> \
>> .flow = &flow,
>> \
>> .support = {
>> \
>> - .NAME = true,
>> \
>> + .SUPPORT = true,
>> \
>> },
>> \
>> };
>> \
>>
>> \
>> memset(&flow, 0, sizeof flow);
>> \
>> - flow.FIELD = 1;
>> \
>> + flow.FIELD = VALUE;
>> \
>>
>> \
>> ofpbuf_use_stack(&key, &keybuf, sizeof keybuf);
>> \
>> odp_flow_key_from_flow(&odp_parms, &key);
>> \
>> @@ -1267,12 +1267,13 @@ check_##NAME(struct dpif_backer *backer)
>> \
>>
>> \
>> return enable;
>> \
>> }
>> -#define CHECK_FEATURE(FIELD) CHECK_FEATURE__(FIELD, FIELD)
>> +#define CHECK_FEATURE(FIELD) CHECK_FEATURE__(FIELD, FIELD, FIELD, 1)
>>
>> CHECK_FEATURE(ct_state)
>> CHECK_FEATURE(ct_zone)
>> CHECK_FEATURE(ct_mark)
>> -CHECK_FEATURE__(ct_label, ct_label.u64.lo)
>> +CHECK_FEATURE__(ct_label, ct_label, ct_label.u64.lo, 1)
>> +CHECK_FEATURE__(ct_state_nat, ct_state, ct_state,
>>CS_TRACKED|CS_SRC_NAT)
>>
>> #undef CHECK_FEATURE
>> #undef CHECK_FEATURE__
>> @@ -1293,6 +1294,8 @@ check_support(struct dpif_backer *backer)
>> backer->support.odp.ct_zone = check_ct_zone(backer);
>> backer->support.odp.ct_mark = check_ct_mark(backer);
>> backer->support.odp.ct_label = check_ct_label(backer);
>> +
>> + backer->support.odp.ct_state_nat = check_ct_state_nat(backer);
>> }
>>
>> static int
>> @@ -4023,7 +4026,7 @@ check_mask(struct ofproto_dpif *ofproto, const
>>struct miniflow *flow)
>> support = &ofproto_dpif_get_support(ofproto)->odp;
>> ct_state = MINIFLOW_GET_U16(flow, ct_state);
>> if (support->ct_state && support->ct_zone && support->ct_mark
>> - && support->ct_label) {
>> + && support->ct_label && support->ct_state_nat) {
>> return ct_state & CS_UNSUPPORTED_MASK ? OFPERR_OFPBMC_BAD_MASK
>>: 0;
>> }
>>
>> @@ -4033,6 +4036,7 @@ check_mask(struct ofproto_dpif *ofproto, const
>>struct miniflow *flow)
>>
>> if ((ct_state && !support->ct_state)
>> || (ct_state & CS_UNSUPPORTED_MASK)
>> + || ((ct_state & (CS_SRC_NAT | CS_DST_NAT)) &&
>>!support->ct_state_nat)
>> || (ct_zone && !support->ct_zone)
>> || (ct_mark && !support->ct_mark)
>> || (!ovs_u128_is_zero(&ct_label) && !support->ct_label)) {
>> @@ -4070,6 +4074,12 @@ check_actions(const struct ofproto_dpif *ofproto,
>> OFPACT_FOR_EACH(a, ct->actions, ofpact_ct_get_action_len(ct)) {
>> const struct mf_field *dst = ofpact_get_mf_dst(a);
>>
>> + if (a->type == OFPACT_NAT && !support->ct_state_nat) {
>> + /* The backer doesn't seem to support the NAT bits in
>> + * 'ct_state': assume that it doesn't support the NAT
>> + * action. */
>> + return OFPERR_OFPBAC_BAD_TYPE;
>> + }
>> if (dst && ((dst->id == MFF_CT_MARK && !support->ct_mark)
>> || (dst->id == MFF_CT_LABEL &&
>>!support->ct_label))) {
>> return OFPERR_OFPBAC_BAD_SET_ARGUMENT;
>> --
>> 2.1.4
>>
>> _______________________________________________
>> dev mailing list
>> dev@openvswitch.org
>>
>>https://urldefense.proofpoint.com/v2/url?u=http-3A__openvswitch.org_mailm
>>an_listinfo_dev&d=BQIFAg&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=
>>SmB5nZacmXNq0gKCC1s_Cw5yUNjxgD4v5kJqZ2uWLlE&m=9_FS9jUZK7ajjQBI3-eSdx7Q_ut
>>Jg86ig52aIZ39fRU&s=D0LZ2r0dC9bkQVANqepfZ5S_6LLGfY-YvcD4POBqlA0&e=
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
