Re: [ovs-dev] [PATCH] ofproto-dpif: Log warning when ct action or its variants are not supported.
On Mon, Oct 31, 2016 at 05:23:25PM -0700, Joe Stringer wrote: > On 31 October 2016 at 14:33, Ben Pfaff wrote: > > On Mon, Oct 31, 2016 at 02:16:05PM -0700, Joe Stringer wrote: > >> On 31 October 2016 at 13:23, Ben Pfaff wrote: > >> > Some datapaths do not support the ct action, and others support only a > >> > subset of its features. Until now, it has been difficult to tell why a > >> > particular action is being rejected. This commit should make it clearer. > >> > > >> > Reported-by: Kevin Lin > >> > Reported-at: > >> > http://openvswitch.org/pipermail/discuss/2016-October/023060.html > >> > Signed-off-by: Ben Pfaff > >> > >> Thanks, no doubt this will save a bunch of people a bunch of > >> confusion. It still doesn't directly state that "Your kernel module > >> may be out of date", but it's more clear than just OpenFlow hexdumps > >> telling you "OFPBAC". Maybe it should either say this, or this should > >> be mentioned in the FAQ. > >> > >> Acked-by: Joe Stringer > > > > You're right, this can be better. > > > > How about like this? > > > > --8<--cut here-->8-- > > > > From: Ben Pfaff > > Date: Mon, 31 Oct 2016 14:33:13 -0700 > > Subject: [PATCH] ofproto-dpif: Log warning when ct action or its variants > > are > > not supported. > > > > Some datapaths do not support the ct action, and others support only a > > subset of its features. Until now, it has been difficult to tell why a > > particular action is being rejected. This commit should make it clearer. > > > > Reported-by: Kevin Lin > > Reported-at: > > http://openvswitch.org/pipermail/discuss/2016-October/023060.html > > Signed-off-by: Ben Pfaff > > Looks good, thanks! Thanks, applied to master. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] ofproto-dpif: Log warning when ct action or its variants are not supported.
On 31 October 2016 at 14:33, Ben Pfaff wrote: > On Mon, Oct 31, 2016 at 02:16:05PM -0700, Joe Stringer wrote: >> On 31 October 2016 at 13:23, Ben Pfaff wrote: >> > Some datapaths do not support the ct action, and others support only a >> > subset of its features. Until now, it has been difficult to tell why a >> > particular action is being rejected. This commit should make it clearer. >> > >> > Reported-by: Kevin Lin >> > Reported-at: >> > http://openvswitch.org/pipermail/discuss/2016-October/023060.html >> > Signed-off-by: Ben Pfaff >> >> Thanks, no doubt this will save a bunch of people a bunch of >> confusion. It still doesn't directly state that "Your kernel module >> may be out of date", but it's more clear than just OpenFlow hexdumps >> telling you "OFPBAC". Maybe it should either say this, or this should >> be mentioned in the FAQ. >> >> Acked-by: Joe Stringer > > You're right, this can be better. > > How about like this? > > --8<--cut here-->8-- > > From: Ben Pfaff > Date: Mon, 31 Oct 2016 14:33:13 -0700 > Subject: [PATCH] ofproto-dpif: Log warning when ct action or its variants are > not supported. > > Some datapaths do not support the ct action, and others support only a > subset of its features. Until now, it has been difficult to tell why a > particular action is being rejected. This commit should make it clearer. > > Reported-by: Kevin Lin > Reported-at: http://openvswitch.org/pipermail/discuss/2016-October/023060.html > Signed-off-by: Ben Pfaff Looks good, thanks! ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] ofproto-dpif: Log warning when ct action or its variants are not supported.
On Mon, Oct 31, 2016 at 02:16:05PM -0700, Joe Stringer wrote:
> On 31 October 2016 at 13:23, Ben Pfaff wrote:
> > Some datapaths do not support the ct action, and others support only a
> > subset of its features. Until now, it has been difficult to tell why a
> > particular action is being rejected. This commit should make it clearer.
> >
> > Reported-by: Kevin Lin
> > Reported-at:
> > http://openvswitch.org/pipermail/discuss/2016-October/023060.html
> > Signed-off-by: Ben Pfaff
>
> Thanks, no doubt this will save a bunch of people a bunch of
> confusion. It still doesn't directly state that "Your kernel module
> may be out of date", but it's more clear than just OpenFlow hexdumps
> telling you "OFPBAC". Maybe it should either say this, or this should
> be mentioned in the FAQ.
>
> Acked-by: Joe Stringer
You're right, this can be better.
How about like this?
--8<--cut here-->8--
From: Ben Pfaff
Date: Mon, 31 Oct 2016 14:33:13 -0700
Subject: [PATCH] ofproto-dpif: Log warning when ct action or its variants are
not supported.
Some datapaths do not support the ct action, and others support only a
subset of its features. Until now, it has been difficult to tell why a
particular action is being rejected. This commit should make it clearer.
Reported-by: Kevin Lin
Reported-at: http://openvswitch.org/pipermail/discuss/2016-October/023060.html
Signed-off-by: Ben Pfaff
---
ofproto/ofproto-dpif.c | 14 ++
1 file changed, 14 insertions(+)
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 7374ccc..330bd48 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -4134,6 +4134,16 @@ check_mask(struct ofproto_dpif *ofproto, const struct
miniflow *flow)
return 0;
}
+static void
+report_unsupported_ct(const char *detail)
+{
+static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
+VLOG_WARN_RL(&rl, "Rejecting ct action because datapath does not support "
+ "ct action%s%s (your kernel module may be out of date)",
+ detail ? " " : "",
+ detail ? detail : "");
+}
+
static enum ofperr
check_actions(const struct ofproto_dpif *ofproto,
const struct rule_actions *const actions)
@@ -4153,9 +4163,11 @@ check_actions(const struct ofproto_dpif *ofproto,
support = &ofproto_dpif_get_support(ofproto)->odp;
if (!support->ct_state) {
+report_unsupported_ct(NULL);
return OFPERR_OFPBAC_BAD_TYPE;
}
if ((ct->zone_imm || ct->zone_src.field) && !support->ct_zone) {
+report_unsupported_ct("zone");
return OFPERR_OFPBAC_BAD_ARGUMENT;
}
@@ -4166,10 +4178,12 @@ check_actions(const struct ofproto_dpif *ofproto,
/* The backer doesn't seem to support the NAT bits in
* 'ct_state': assume that it doesn't support the NAT
* action. */
+report_unsupported_ct("nat");
return OFPERR_OFPBAC_BAD_TYPE;
}
if (dst && ((dst->id == MFF_CT_MARK && !support->ct_mark)
|| (dst->id == MFF_CT_LABEL && !support->ct_label))) {
+report_unsupported_ct("setting mark and/or label");
return OFPERR_OFPBAC_BAD_SET_ARGUMENT;
}
}
--
2.1.3
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] ofproto-dpif: Log warning when ct action or its variants are not supported.
On 31 October 2016 at 13:23, Ben Pfaff wrote: > Some datapaths do not support the ct action, and others support only a > subset of its features. Until now, it has been difficult to tell why a > particular action is being rejected. This commit should make it clearer. > > Reported-by: Kevin Lin > Reported-at: http://openvswitch.org/pipermail/discuss/2016-October/023060.html > Signed-off-by: Ben Pfaff Thanks, no doubt this will save a bunch of people a bunch of confusion. It still doesn't directly state that "Your kernel module may be out of date", but it's more clear than just OpenFlow hexdumps telling you "OFPBAC". Maybe it should either say this, or this should be mentioned in the FAQ. Acked-by: Joe Stringer ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
