[ovs-dev] [PATCH v2] lib: More intuitive syntax for TCP flags matching.

Jarno Rajahalme jrajahalme at nicira.com
Tue Nov 26 17:04:41 UTC 2013


Allow TCP flags match specification with symbolic flag names.  TCP
flags are optionally specified as a string of flag names, each
preceded by '+' when the flag must be one, or '-' when the flag must
be zero.  Any flags not explicitly included are wildcarded.  The
existing hex syntax is still allowed, and is used in flow dumps when
all the flags are matched.

Signed-off-by: Jarno Rajahalme <jrajahalme at nicira.com>
---
v2: Addressed Ben's comments.

 lib/flow.c               |   18 +++++++++
 lib/flow.h               |    3 ++
 lib/match.c              |    7 ++--
 lib/meta-flow.c          |   94 +++++++++++++++++++++++++++++++++++++++++++++-
 lib/meta-flow.h          |    1 +
 lib/packets.c            |   33 ++++++++++++++++
 lib/packets.h            |    1 +
 tests/ovs-ofctl.at       |   16 +++++---
 utilities/ovs-ofctl.8.in |   28 +++++++++-----
 9 files changed, 182 insertions(+), 19 deletions(-)

diff --git a/lib/flow.c b/lib/flow.c
index c6683a5..67003b3 100644
--- a/lib/flow.c
+++ b/lib/flow.c
@@ -587,6 +587,24 @@ format_flags(struct ds *ds, const char *(*bit_to_string)(uint32_t),
 }
 
 void
+format_flags_masked(struct ds *ds, const char *name,
+                    const char *(*bit_to_string)(uint32_t), uint32_t flags,
+                    uint32_t mask)
+{
+    if (name) {
+        ds_put_format(ds, "%s=", name);
+    }
+    while (mask) {
+        uint32_t bit = rightmost_1bit(mask);
+        const char *s = bit_to_string(bit);
+
+        ds_put_format(ds, "%s%s", (flags & bit) ? "+" : "-",
+                      s ? s : "[Unknown]");
+        mask &= ~bit;
+    }
+}
+
+void
 flow_format(struct ds *ds, const struct flow *flow)
 {
     struct match match;
diff --git a/lib/flow.h b/lib/flow.h
index 5e78073..5c0007d 100644
--- a/lib/flow.h
+++ b/lib/flow.h
@@ -179,6 +179,9 @@ void flow_get_metadata(const struct flow *, struct flow_metadata *);
 char *flow_to_string(const struct flow *);
 void format_flags(struct ds *ds, const char *(*bit_to_string)(uint32_t),
                   uint32_t flags, char del);
+void format_flags_masked(struct ds *ds, const char *name,
+                         const char *(*bit_to_string)(uint32_t),
+                         uint32_t flags, uint32_t mask);
 
 void flow_format(struct ds *, const struct flow *);
 void flow_print(FILE *, const struct flow *);
diff --git a/lib/match.c b/lib/match.c
index 71d86be..1154b20 100644
--- a/lib/match.c
+++ b/lib/match.c
@@ -1069,11 +1069,12 @@ match_format(const struct match *match, struct ds *s, unsigned int priority)
         format_be16_masked(s, "tp_dst", f->tp_dst, wc->masks.tp_dst);
     }
     if (is_ip_any(f) && f->nw_proto == IPPROTO_TCP && wc->masks.tcp_flags) {
-        if (wc->masks.tcp_flags == htons(UINT16_MAX)) {
+        uint16_t mask = TCP_FLAGS(wc->masks.tcp_flags);
+        if (mask == TCP_FLAGS(OVS_BE16_MAX)) {
             ds_put_format(s, "tcp_flags=0x%03"PRIx16",", ntohs(f->tcp_flags));
         } else {
-            ds_put_format(s, "tcp_flags=0x%03"PRIx16"/0x%03"PRIx16",",
-                          ntohs(f->tcp_flags), ntohs(wc->masks.tcp_flags));
+            format_flags_masked(s, "tcp_flags", packet_tcp_flag_to_string,
+                                ntohs(f->tcp_flags), mask);
         }
     }
 
diff --git a/lib/meta-flow.c b/lib/meta-flow.c
index b37f781..9aaef6e 100644
--- a/lib/meta-flow.c
+++ b/lib/meta-flow.c
@@ -565,7 +565,7 @@ const struct mf_field mf_fields[MFF_N_IDS] = {
         MFF_TCP_FLAGS, "tcp_flags", NULL,
         2, 12,
         MFM_FULLY,
-        MFS_HEXADECIMAL,
+        MFS_TCP_FLAGS,
         MFP_TCP,
         false,
         NXM_NX_TCP_FLAGS, "NXM_NX_TCP_FLAGS",
@@ -2595,6 +2595,81 @@ mf_from_tun_flags_string(const char *s, ovs_be16 *valuep, ovs_be16 *maskp)
                      "\"csum\", \"key\")", s);
 }
 
+static char *
+mf_from_tcp_flags_string(const char *s, ovs_be16 *flagsp, ovs_be16 *maskp)
+{
+    uint16_t flags = 0;
+    uint16_t mask = 0;
+    uint16_t bit;
+    int n;
+
+    if (ovs_scan(s, "%"SCNi16"/%"SCNi16"%n", &flags, &mask, &n) && !s[n]) {
+        *flagsp = htons(flags);
+        *maskp = htons(mask);
+        return NULL;
+    }
+    if (ovs_scan(s, "%"SCNi16"%n", &flags, &n) && !s[n]) {
+        *flagsp = htons(flags);
+        *maskp = OVS_BE16_MAX;
+        return NULL;
+    }
+
+    while (*s != '\0') {
+        bool set;
+        int name_len;
+
+        switch (*s) {
+        case '+':
+            set = true;
+            break;
+        case '-':
+            set = false;
+            break;
+        default:
+            return xasprintf("%s: TCP flag must be preceded by '+' (for SET) "
+                             "or '-' (NOT SET)", s);
+        }
+        s++;
+
+        name_len = strcspn(s,"+-");
+
+        for (bit = 1; bit; bit <<= 1) {
+            const char *fname = packet_tcp_flag_to_string(bit);
+            size_t len;
+
+            if (!fname) {
+                continue;
+            }
+
+            len = strlen(fname);
+            if (len != name_len) {
+                continue;
+            }
+            if (!strncmp(s, fname, len)) {
+                if (mask & bit) {
+                    return xasprintf("%s: Each TCP flag can be specified only "
+                                     "once", s);
+                }
+                if (set) {
+                    flags |= bit;
+                }
+                mask |= bit;
+                break;
+            }
+        }
+
+        if (!bit) {
+            return xasprintf("%s: unknown TCP flag(s)", s);
+        }
+        s += name_len;
+    }
+
+    *flagsp = htons(flags);
+    *maskp = htons(mask);
+    return NULL;
+}
+
+
 /* Parses 's', a string value for field 'mf', into 'value' and 'mask'.  Returns
  * NULL if successful, otherwise a malloc()'d string describing the error. */
 char *
@@ -2645,6 +2720,11 @@ mf_parse(const struct mf_field *mf, const char *s,
         error = mf_from_tun_flags_string(s, &value->be16, &mask->be16);
         break;
 
+    case MFS_TCP_FLAGS:
+        ovs_assert(mf->n_bytes == sizeof(ovs_be16));
+        error = mf_from_tcp_flags_string(s, &value->be16, &mask->be16);
+        break;
+
     default:
         NOT_REACHED();
     }
@@ -2731,6 +2811,13 @@ mf_format_tnl_flags_string(const ovs_be16 *valuep, struct ds *s)
     format_flags(s, flow_tun_flag_to_string, ntohs(*valuep), '|');
 }
 
+static void
+mf_format_tcp_flags_string(ovs_be16 value, ovs_be16 mask, struct ds *s)
+{
+    format_flags_masked(s, NULL, packet_tcp_flag_to_string, ntohs(value),
+                        TCP_FLAGS(mask));
+}
+
 /* Appends to 's' a string representation of field 'mf' whose value is in
  * 'value' and 'mask'.  'mask' may be NULL to indicate an exact match. */
 void
@@ -2787,6 +2874,11 @@ mf_format(const struct mf_field *mf,
         mf_format_tnl_flags_string(&value->be16, s);
         break;
 
+    case MFS_TCP_FLAGS:
+        mf_format_tcp_flags_string(value->be16,
+                                   mask ? mask->be16 : OVS_BE16_MAX, s);
+        break;
+
     default:
         NOT_REACHED();
     }
diff --git a/lib/meta-flow.h b/lib/meta-flow.h
index c2b0bbc..b43b13f 100644
--- a/lib/meta-flow.h
+++ b/lib/meta-flow.h
@@ -232,6 +232,7 @@ enum OVS_PACKED_ENUM mf_string {
     MFS_OFP_PORT_OXM,           /* An OpenFlow port number or name (32-bit). */
     MFS_FRAG,                   /* no, yes, first, later, not_later */
     MFS_TNL_FLAGS,              /* FLOW_TNL_F_* flags */
+    MFS_TCP_FLAGS,              /* TCP_* flags */
 };
 
 struct mf_field {
diff --git a/lib/packets.c b/lib/packets.c
index 4bec4d1..095908f 100644
--- a/lib/packets.c
+++ b/lib/packets.c
@@ -909,6 +909,39 @@ packet_get_tcp_flags(const struct ofpbuf *packet, const struct flow *flow)
     }
 }
 
+const char *
+packet_tcp_flag_to_string(uint32_t flag)
+{
+    switch (flag) {
+    case TCP_FIN:
+        return "fin";
+    case TCP_SYN:
+        return "syn";
+    case TCP_RST:
+        return "rst";
+    case TCP_PSH:
+        return "psh";
+    case TCP_ACK:
+        return "ack";
+    case TCP_URG:
+        return "urg";
+    case TCP_ECE:
+        return "ece";
+    case TCP_CWR:
+        return "cwr";
+    case TCP_NS:
+        return "ns";
+    case 0x200:
+        return "[200]";
+    case 0x400:
+        return "[400]";
+    case 0x800:
+        return "[800]";
+    default:
+        return NULL;
+    }
+}
+
 /* Appends a string representation of the TCP flags value 'tcp_flags'
  * (e.g. obtained via packet_get_tcp_flags() or TCP_FLAGS) to 's', in the
  * format used by tcpdump. */
diff --git a/lib/packets.h b/lib/packets.h
index ef8c00c..d291c14 100644
--- a/lib/packets.h
+++ b/lib/packets.h
@@ -643,5 +643,6 @@ void packet_set_sctp_port(struct ofpbuf *, ovs_be16 src, ovs_be16 dst);
 
 uint16_t packet_get_tcp_flags(const struct ofpbuf *, const struct flow *);
 void packet_format_tcp_flags(struct ds *, uint16_t);
+const char *packet_tcp_flag_to_string(uint32_t flag);
 
 #endif /* packets.h */
diff --git a/tests/ovs-ofctl.at b/tests/ovs-ofctl.at
index 3434592..6e6461b 100644
--- a/tests/ovs-ofctl.at
+++ b/tests/ovs-ofctl.at
@@ -2470,23 +2470,27 @@ AT_SETUP([tcp flags - filtering])
 OVS_VSWITCHD_START([add-port br0 p1 -- set Interface p1 type=dummy ofport_request=1 \
                     -- add-port br0 p2 -- set Interface p2 type=dummy ofport_request=2])
 AT_DATA([flows.txt], [dnl
-	in_port=1,tcp,tp_dst=80,tcp_flags=0x02/0x17,action=2  # Allow outbound web traffic bare-SYN
-	in_port=1,tcp,tp_dst=80,tcp_flags=0x10/0x10,action=2  # Allow outbound web traffic with ACK bit
-	in_port=1,tcp,tp_dst=80,tcp_flags=0x04/0x04,action=2  # Allow outbound web traffic with RST bit
-	in_port=2,tcp,tp_src=80,tcp_flags=0x10/0x10,action=1  # Allow inbound web traffic with ACK bit
-	in_port=2,tcp,tp_src=80,tcp_flags=0x04/0x04,action=1  # Allow inbound web traffic with RST bit
+	in_port=1,tcp,tp_dst=80,tcp_flags=+syn-rst-ack-fin,action=2  # Allow outbound web traffic bare-SYN
+	in_port=1,tcp,tp_dst=80,tcp_flags=+ack,action=2  # Allow outbound web traffic with ACK bit
+	in_port=1,tcp,tp_dst=80,tcp_flags=+rst,action=2  # Allow outbound web traffic with RST bit
+	in_port=2,tcp,tp_src=80,tcp_flags=+ack,action=1  # Allow inbound web traffic with ACK bit
+	in_port=2,tcp,tp_src=80,tcp_flags=+rst,action=1  # Allow inbound web traffic with RST bit
 	priority=0,in_port=1,action=drop  # Default drop outbound
 	priority=0,in_port=2,action=drop  # Default drop inbound
 ])
 
 AT_CHECK([ovs-ofctl add-flows br0 flows.txt])
 
+AT_CHECK([ovs-ofctl add-flow br0 "tcp,tcp_flags=+ack-ack,action="], [1], [],
+  [ovs-ofctl: ack: Each TCP flag can be specified only once
+])
+
 AT_CHECK([ovs-appctl dpif/show | tail -n +5], [0], [dnl
 		p1 1/1: (dummy)
 		p2 2/2: (dummy)
 ])
 
-dnl Outbound web traffic with base-SYN
+dnl Outbound web traffic with bare-SYN
 AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'in_port(1),eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:07),eth_type(0x0800),ipv4(src=192.168.0.1,dst=192.168.0.2,proto=6,tos=0,ttl=64,frag=no),tcp(src=8,dst=80),tcp_flags(0x002)'], [0], [stdout])
 AT_CHECK([tail -1 stdout], [0],
   [Datapath actions: 2
diff --git a/utilities/ovs-ofctl.8.in b/utilities/ovs-ofctl.8.in
index e5e488a..d60d871 100644
--- a/utilities/ovs-ofctl.8.in
+++ b/utilities/ovs-ofctl.8.in
@@ -728,33 +728,43 @@ above, the bitwise match forms apply only when \fBdl_type\fR and
 \fBnw_proto\fR specify TCP or UDP or SCTP.
 .
 .IP \fBtcp_flags=\fIflags\fB/\fImask\fR
+.IQ \fBtcp_flags=\fR[\fB+\fIflag\fR...][\fB-\fIflag\fR...]
 Bitwise match on TCP flags.  The \fIflags\fR and \fImask\fR are 16-bit
 numbers written in decimal or in hexadecimal prefixed by \fB0x\fR.
 Each 1-bit in \fImask\fR requires that the corresponding bit in
 \fIflags\fR must match.  Each 0-bit in \fImask\fR causes the corresponding
 bit to be ignored.
 .IP
+Alternatively, the flags can be specified by their symbolic names (listed
+below), each preceded by either a '+' (plus sign) for a flag that must be
+set, or a '-' (minus sign) for a flag that must be reset, without any other
+delimiters between the flags.  Any flags not explicitly included are
+wildcarded.  For example:
+.RS
+.IP "\fBtcp_flags=+syn-ack\fR"
+.RE
+.IP
 TCP protocol currently defines 9 flag bits, and additional 3 bits are
 reserved (must be transmitted as zero), see RFCs 793, 3168, and 3540.
 The flag bits are, numbering from the least significant bit:
 .RS
-.IP "\fB0: FIN\fR"
+.IP "\fB0: fin\fR"
 No more data from sender.
-.IP "\fB1: SYN\fR"
+.IP "\fB1: syn\fR"
 Synchronize sequence numbers.
-.IP "\fB2: RST\fR"
+.IP "\fB2: rst\fR"
 Reset the connection.
-.IP "\fB3: PSH\fR"
+.IP "\fB3: psh\fR"
 Push function.
-.IP "\fB4: ACK\fR"
+.IP "\fB4: ack\fR"
 Acknowledgement field significant.
-.IP "\fB5: URG\fR"
+.IP "\fB5: urg\fR"
 Urgent pointer field significant.
-.IP "\fB6: ECE\fR"
+.IP "\fB6: ece\fR"
 ECN Echo.
-.IP "\fB7: CWR\fR"
+.IP "\fB7: cwr\fR"
 Congestion Windows Reduced.
-.IP "\fB8: NS\fR"
+.IP "\fB8: ns\fR"
 Nonce Sum.
 .IP "\fB9-11:\fR"
 Reserved.
-- 
1.7.10.4




More information about the dev mailing list