I spent way too much time this last week tracking down errors due to a VM with an antique kernel module that didn't support connection tracking. This commit adds clear error messages that would have made the problem obvious.
Signed-off-by: Ben Pfaff <[email protected]> --- include/openvswitch/ofp-errors.h | 12 ++++++++++++ ofproto/ofproto-dpif.c | 27 +++++++++++++++------------ 2 files changed, 27 insertions(+), 12 deletions(-) diff --git a/include/openvswitch/ofp-errors.h b/include/openvswitch/ofp-errors.h index 5e20e1adb7cd..283b9af40248 100644 --- a/include/openvswitch/ofp-errors.h +++ b/include/openvswitch/ofp-errors.h @@ -287,6 +287,12 @@ enum ofperr { /* NX1.3+(41). Error in encap or decap property. */ OFPERR_NXBAC_BAD_ED_PROP, + /* NX1.0-1.1(1,265), NX1.2+(42). Action requires connection tracking or a + * particular connection-tracking based feature that the datapath in use + * does not support. If a kernel-based datapath is in use, the kernel + * module may need to be upgraded. */ + OFPERR_NXBAC_CT_DATAPATH_SUPPORT, + /* ## --------------------- ## */ /* ## OFPET_BAD_INSTRUCTION ## */ /* ## --------------------- ## */ @@ -367,6 +373,12 @@ enum ofperr { /* OF1.2+(4,11). Permissions error. */ OFPERR_OFPBMC_EPERM, + /* NX1.0-1.1(1,264), NX1.2+(43). Flow match requires connection tracking + * or a particular connection-tracking based feature that the datapath in + * use does not support. If a kernel-based datapath is in use, the kernel + * module may need to be upgraded. */ + OFPERR_NXBMC_CT_DATAPATH_SUPPORT, + /* ## --------------------- ## */ /* ## OFPET_FLOW_MOD_FAILED ## */ /* ## --------------------- ## */ diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c index 838a8de0c27f..da0c64671880 100644 --- a/ofproto/ofproto-dpif.c +++ b/ofproto/ofproto-dpif.c @@ -4193,12 +4193,16 @@ check_mask(struct ofproto_dpif *ofproto, const struct miniflow *flow) support = &ofproto->backer->rt_support.odp; ct_state = MINIFLOW_GET_U8(flow, ct_state); + if (ct_state & CS_UNSUPPORTED_MASK) { + return OFPERR_OFPBMC_BAD_MASK; + } + /* Do not bother dissecting the flow further if the datapath supports all * the features we know of. */ if (support->ct_state && support->ct_zone && support->ct_mark && support->ct_label && support->ct_state_nat && support->ct_orig_tuple && support->ct_orig_tuple6) { - return ct_state & CS_UNSUPPORTED_MASK ? OFPERR_OFPBMC_BAD_MASK : 0; + return 0; } ct_zone = MINIFLOW_GET_U16(flow, ct_zone); @@ -4206,31 +4210,30 @@ check_mask(struct ofproto_dpif *ofproto, const struct miniflow *flow) ct_label = MINIFLOW_GET_U128(flow, ct_label); 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)) { - return OFPERR_OFPBMC_BAD_MASK; + return OFPERR_NXBMC_CT_DATAPATH_SUPPORT; } if (!support->ct_orig_tuple && !support->ct_orig_tuple6 && (MINIFLOW_GET_U8(flow, ct_nw_proto) || MINIFLOW_GET_U16(flow, ct_tp_src) || MINIFLOW_GET_U16(flow, ct_tp_dst))) { - return OFPERR_OFPBMC_BAD_MASK; + return OFPERR_NXBMC_CT_DATAPATH_SUPPORT; } if (!support->ct_orig_tuple && (MINIFLOW_GET_U32(flow, ct_nw_src) || MINIFLOW_GET_U32(flow, ct_nw_dst))) { - return OFPERR_OFPBMC_BAD_MASK; + return OFPERR_NXBMC_CT_DATAPATH_SUPPORT; } if (!support->ct_orig_tuple6 && (!ovs_u128_is_zero(MINIFLOW_GET_U128(flow, ct_ipv6_src)) || !ovs_u128_is_zero(MINIFLOW_GET_U128(flow, ct_ipv6_dst)))) { - return OFPERR_OFPBMC_BAD_MASK; + return OFPERR_NXBMC_CT_DATAPATH_SUPPORT; } return 0; @@ -4261,11 +4264,11 @@ check_actions(const struct ofproto_dpif *ofproto, if (!support->ct_state) { report_unsupported_act("ct", "ct action"); - return OFPERR_OFPBAC_BAD_TYPE; + return OFPERR_NXBAC_CT_DATAPATH_SUPPORT; } if ((ct->zone_imm || ct->zone_src.field) && !support->ct_zone) { report_unsupported_act("ct", "ct zones"); - return OFPERR_OFPBAC_BAD_ARGUMENT; + return OFPERR_NXBAC_CT_DATAPATH_SUPPORT; } /* So far the force commit feature is implemented together with the * original direction tuple feature by all datapaths, so we use the @@ -4273,7 +4276,7 @@ check_actions(const struct ofproto_dpif *ofproto, * force commit feature as well. */ if ((ct->flags & NX_CT_F_FORCE) && !support->ct_orig_tuple) { report_unsupported_act("ct", "force commit"); - return OFPERR_OFPBAC_BAD_ARGUMENT; + return OFPERR_NXBAC_CT_DATAPATH_SUPPORT; } OFPACT_FOR_EACH(a, ct->actions, ofpact_ct_get_action_len(ct)) { @@ -4284,12 +4287,12 @@ check_actions(const struct ofproto_dpif *ofproto, * 'ct_state': assume that it doesn't support the NAT * action. */ report_unsupported_act("ct", "nat"); - return OFPERR_OFPBAC_BAD_TYPE; + return OFPERR_NXBAC_CT_DATAPATH_SUPPORT; } if (dst && ((dst->id == MFF_CT_MARK && !support->ct_mark) || (dst->id == MFF_CT_LABEL && !support->ct_label))) { report_unsupported_act("ct", "setting mark and/or label"); - return OFPERR_OFPBAC_BAD_SET_ARGUMENT; + return OFPERR_NXBAC_CT_DATAPATH_SUPPORT; } } } else if (ofpact->type == OFPACT_RESUBMIT) { @@ -4298,7 +4301,7 @@ check_actions(const struct ofproto_dpif *ofproto, if (resubmit->with_ct_orig && !support->ct_orig_tuple) { report_unsupported_act("resubmit", "ct original direction tuple"); - return OFPERR_OFPBAC_BAD_TYPE; + return OFPERR_NXBAC_CT_DATAPATH_SUPPORT; } } } -- 2.10.2 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
