[ovs-dev] [PATCH] ofproto-dpif: Log warning when ct action or its variants are not supported.

Ben Pfaff blp at ovn.org
Mon Oct 31 21:33:50 UTC 2016


On Mon, Oct 31, 2016 at 02:16:05PM -0700, Joe Stringer wrote:
> On 31 October 2016 at 13:23, Ben Pfaff <blp at ovn.org> 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 <kevinlin at berkeley.edu>
> > Reported-at: http://openvswitch.org/pipermail/discuss/2016-October/023060.html
> > Signed-off-by: Ben Pfaff <blp at ovn.org>
> 
> 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 <joe at ovn.org>

You're right, this can be better.

How about like this?

--8<--------------------------cut here-------------------------->8--

From: Ben Pfaff <blp at ovn.org>
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 <kevinlin at berkeley.edu>
Reported-at: http://openvswitch.org/pipermail/discuss/2016-October/023060.html
Signed-off-by: Ben Pfaff <blp at ovn.org>
---
 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




More information about the dev mailing list