On 1/10/18, 1:05 PM, "Ben Pfaff" <[email protected]> wrote:
On Wed, Jan 10, 2018 at 08:49:15PM +0000, Darrell Ball wrote:
> Thanks for doing this; I have debugged the associated kinds of problems
and the new more granular errors for mask and action will help.
>
> I did not test it yet, but I have one comment inline.
Thanks for the comments and the positive words!
> On 1/10/18, 8:35 AM, "[email protected] on behalf of Ben
Pfaff" <[email protected] on behalf of [email protected]> wrote:
>
> 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]>
> 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;
> + }
> +
>
> [Darrell] Why do we still use OFPERR_OFPBMC_BAD_MASK above, rather
> than the new OFPERR_NXBMC_CT_DATAPATH_SUPPORT ?
That's because this test is different from the others. Rather than
checking for a feature that is not supported in the datapath, it checks
whether the controller is trying to use a feature that has never been
defined and OVS doesn't know about. This might be caused, for example,
by a controller trying to use a connection tracking feature that only
exists in a newer version of OVS than the one running.
I agree - it is a different error under the ‘CT class’ of errors.
I guess we could invent another error code, something like
OFPBMC_UNKNOWN_CT_FEATURE, if we think that this is a likely problem.
I don’t know how likely it is, but I would guess it can happen in general.
In this particular case, it is less likely, hence I am fine either way.
Acked-by: Darrell Ball <[email protected]>
Btw: All regression tests passed.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
