[ovs-dev,v2] conntrack: add display support for sctp
diff mbox series

Message ID 20190517190549.29669-1-aconole@redhat.com
State New
Headers show
Series
  • [ovs-dev,v2] conntrack: add display support for sctp
Related show

Commit Message

Aaron Conole May 17, 2019, 7:05 p.m. UTC
Currently, only the netlink datapath supports SCTP connection tracking,
but at least this removes the warning message that will pop up when
running something like:

   ovs-appctl dpctl/dump-conntrack

This doesn't impact any conntrack functionality, just the display.

Signed-off-by: Aaron Conole <aconole@redhat.com>
---
 acinclude.m4            | 12 +++++++
 configure.ac            |  1 +
 lib/ct-dpif.c           | 19 +++++++++++
 lib/ct-dpif.h           | 32 ++++++++++++++++++
 lib/netlink-conntrack.c | 72 ++++++++++++++++++++++++++++++++++++++++-
 5 files changed, 135 insertions(+), 1 deletion(-)

Comments

Ben Pfaff May 20, 2019, 11:30 p.m. UTC | #1
On Fri, May 17, 2019 at 03:05:49PM -0400, Aaron Conole wrote:
> Currently, only the netlink datapath supports SCTP connection tracking,
> but at least this removes the warning message that will pop up when
> running something like:
> 
>    ovs-appctl dpctl/dump-conntrack
> 
> This doesn't impact any conntrack functionality, just the display.
> 
> Signed-off-by: Aaron Conole <aconole@redhat.com>

With this patch, some OVS binaries will support displaying SCTP
connection tracking information, and some will not, depending on the
kernel headers that OVS was built against.  These kernel headers hardly
ever are from the same version of the kernel that the user actually
boots, so the user might end up running OVS without SCTP connection
tracking display on top of Linux with SCTP connection tracking.  This is
confusing.

In this kind of situation, when we can, we try to compensate for missing
or old headers or other definitions at compile time, so that OVS
supports all the kernel features regardless of the headers it was
compiled against.  Commit 1dc0da67b000 ("compat: Add tc compatibility
headers for old kernels") gives an example of how to do this in a
similar situation.

Will you please consider how to do this for sctp conntrack display also?

Thanks,

Ben.
Aaron Conole May 21, 2019, 1:49 p.m. UTC | #2
Ben Pfaff <blp@ovn.org> writes:

> On Fri, May 17, 2019 at 03:05:49PM -0400, Aaron Conole wrote:
>> Currently, only the netlink datapath supports SCTP connection tracking,
>> but at least this removes the warning message that will pop up when
>> running something like:
>> 
>>    ovs-appctl dpctl/dump-conntrack
>> 
>> This doesn't impact any conntrack functionality, just the display.
>> 
>> Signed-off-by: Aaron Conole <aconole@redhat.com>
>
> With this patch, some OVS binaries will support displaying SCTP
> connection tracking information, and some will not, depending on the
> kernel headers that OVS was built against.  These kernel headers hardly
> ever are from the same version of the kernel that the user actually
> boots, so the user might end up running OVS without SCTP connection
> tracking display on top of Linux with SCTP connection tracking.  This is
> confusing.
>
> In this kind of situation, when we can, we try to compensate for missing
> or old headers or other definitions at compile time, so that OVS
> supports all the kernel features regardless of the headers it was
> compiled against.  Commit 1dc0da67b000 ("compat: Add tc compatibility
> headers for old kernels") gives an example of how to do this in a
> similar situation.
>
> Will you please consider how to do this for sctp conntrack display also?

Sure thing.

> Thanks,
>
> Ben.

Patch
diff mbox series

diff --git a/acinclude.m4 b/acinclude.m4
index f8fc5bcd7..7ca2c5716 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -212,6 +212,18 @@  AC_DEFUN([OVS_CHECK_LINUX_TC], [
                [Define to 1 if TCA_SKBEDIT_FLAGS is available.])])
 ])
 
+dnl OVS_CHECK_LINUX_SCTP_CT
+dnl
+dnl Checks for kernels which need additional SCTP state
+AC_DEFUN([OVS_CHECK_LINUX_SCTP_CT], [
+  AC_COMPILE_IFELSE([
+    AC_LANG_PROGRAM([#include <linux/netfilter/nf_conntrack_sctp.h], [
+        int x = SCTP_CONNTRACK_HEARTBEAT_SENT;
+    ])],
+    [AC_DEFINE([HAVE_SCTP_CONNTRACK_HEARTBEATS], [1],
+               [Define to 1 if SCTP_CONNTRACK_HEARTBEAT_SENT is available.])])
+])
+
 dnl OVS_FIND_DEPENDENCY(FUNCTION, SEARCH_LIBS, NAME_TO_PRINT)
 dnl
 dnl Check for a function in a library list.
diff --git a/configure.ac b/configure.ac
index 505e3d041..2dbe9a917 100644
--- a/configure.ac
+++ b/configure.ac
@@ -186,6 +186,7 @@  AC_ARG_VAR(KARCH, [Kernel Architecture String])
 AC_SUBST(KARCH)
 OVS_CHECK_LINUX
 OVS_CHECK_LINUX_TC
+OVS_CHECK_LINUX_SCTP_CT
 OVS_CHECK_DPDK
 OVS_CHECK_PRAGMA_MESSAGE
 AC_SUBST([OVS_CFLAGS])
diff --git a/lib/ct-dpif.c b/lib/ct-dpif.c
index b2c9b4309..17e348ea9 100644
--- a/lib/ct-dpif.c
+++ b/lib/ct-dpif.c
@@ -428,6 +428,12 @@  const char *ct_dpif_tcp_state_string[] = {
 #undef CT_DPIF_TCP_STATE
 };
 
+const char *ct_dpif_sctp_state_string[] = {
+#define CT_DPIF_SCTP_STATE(STATE) [CT_DPIF_SCTP_STATE_##STATE] = #STATE,
+    CT_DPIF_SCTP_STATES
+#undef CT_DPIF_SCTP_STATE
+};
+
 static void
 ct_dpif_format_enum__(struct ds *ds, const char *title, unsigned int state,
                       const char *names[], unsigned int max)
@@ -497,6 +503,16 @@  ct_dpif_format_protoinfo_tcp_verbose(struct ds *ds,
                          tcp_flags);
 }
 
+static void
+ct_dpif_format_protoinfo_sctp(struct ds *ds,
+                              const struct ct_dpif_protoinfo *protoinfo)
+{
+    ct_dpif_format_enum(ds, "state=", protoinfo->sctp.state,
+                        ct_dpif_sctp_state_string);
+    ds_put_format(ds, ",vtag_orig=%u,vtag_reply=%u", protoinfo->sctp.vtag_orig,
+                  protoinfo->sctp.vtag_reply);
+}
+
 static void
 ct_dpif_format_protoinfo(struct ds *ds, const char *title,
                          const struct ct_dpif_protoinfo *protoinfo,
@@ -514,6 +530,9 @@  ct_dpif_format_protoinfo(struct ds *ds, const char *title,
                 ct_dpif_format_protoinfo_tcp(ds, protoinfo);
             }
             break;
+        case IPPROTO_SCTP:
+            ct_dpif_format_protoinfo_sctp(ds, protoinfo);
+            break;
         }
         if (title) {
             ds_put_cstr(ds, ")");
diff --git a/lib/ct-dpif.h b/lib/ct-dpif.h
index 2628c2b68..93d738c2b 100644
--- a/lib/ct-dpif.h
+++ b/lib/ct-dpif.h
@@ -101,6 +101,33 @@  enum ct_dpif_tcp_flags {
 #undef CT_DPIF_TCP_FLAG
 };
 
+#ifndef HAVE_SCTP_CONNTRACK_HEARTBEATS
+#define CT_DPIF_SCTP_HEARTBEAT_STATES
+#else
+#define CT_DPIF_SCTP_HEARTBEAT_STATES \
+    CT_DPIF_SCTP_STATE(HEARTBEAT_SENT) \
+    CT_DPIF_SCTP_STATE(HEARTBEAT_ACKED)
+#endif
+
+extern const char *ct_dpif_sctp_state_string[];
+
+#define CT_DPIF_SCTP_STATES \
+    CT_DPIF_SCTP_STATE(CLOSED) \
+    CT_DPIF_SCTP_STATE(COOKIE_WAIT) \
+    CT_DPIF_SCTP_STATE(COOKIE_ECHOED) \
+    CT_DPIF_SCTP_STATE(ESTABLISHED) \
+    CT_DPIF_SCTP_STATE(SHUTDOWN_SENT) \
+    CT_DPIF_SCTP_STATE(SHUTDOWN_RECD) \
+    CT_DPIF_SCTP_STATE(SHUTDOWN_ACK_SENT) \
+    CT_DPIF_SCTP_HEARTBEAT_STATES \
+    CT_DPIF_SCTP_STATE(MAX_NUM)
+
+enum ct_dpif_sctp_state {
+#define CT_DPIF_SCTP_STATE(STATE) CT_DPIF_SCTP_STATE_##STATE,
+    CT_DPIF_SCTP_STATES
+#undef CT_DPIF_SCTP_STATE
+};
+
 struct ct_dpif_protoinfo {
     uint16_t proto; /* IPPROTO_* */
     union {
@@ -112,6 +139,11 @@  struct ct_dpif_protoinfo {
             uint8_t flags_orig;
             uint8_t flags_reply;
         } tcp;
+        struct {
+            uint8_t state;
+            uint32_t vtag_orig;
+            uint32_t vtag_reply;
+        } sctp;
     };
 };
 
diff --git a/lib/netlink-conntrack.c b/lib/netlink-conntrack.c
index 42be1d9ce..b833683de 100644
--- a/lib/netlink-conntrack.c
+++ b/lib/netlink-conntrack.c
@@ -717,6 +717,75 @@  nl_ct_parse_protoinfo_tcp(struct nlattr *nla,
     return parsed;
 }
 
+/* Translate netlink SCTP state to CT_DPIF_SCTP state. */
+static uint8_t
+nl_ct_sctp_state_to_dpif(uint8_t state)
+{
+#ifdef _WIN32
+    /* For now, return the CT_DPIF_SCTP state. Not sure what windows does. */
+    return state;
+#else
+    switch (state) {
+    case SCTP_CONNTRACK_COOKIE_WAIT:
+        return CT_DPIF_SCTP_STATE_COOKIE_WAIT;
+    case SCTP_CONNTRACK_COOKIE_ECHOED:
+        return CT_DPIF_SCTP_STATE_COOKIE_ECHOED;
+    case SCTP_CONNTRACK_ESTABLISHED:
+        return CT_DPIF_SCTP_STATE_ESTABLISHED;
+    case SCTP_CONNTRACK_SHUTDOWN_SENT:
+        return CT_DPIF_SCTP_STATE_SHUTDOWN_SENT;
+    case SCTP_CONNTRACK_SHUTDOWN_RECD:
+        return CT_DPIF_SCTP_STATE_SHUTDOWN_RECD;
+    case SCTP_CONNTRACK_SHUTDOWN_ACK_SENT:
+        return CT_DPIF_SCTP_STATE_SHUTDOWN_ACK_SENT;
+#ifdef HAVE_SCTP_CONNTRACK_HEARTBEATS
+    case SCTP_CONNTRACK_HEARTBEAT_SENT:
+        return CT_DPIF_SCTP_STATE_HEARTBEAT_SENT;
+    case SCTP_CONNTRACK_HEARTBEAT_ACKED:
+        return CT_DPIF_SCTP_STATE_HEARTBEAT_ACKED;
+#endif
+    case SCTP_CONNTRACK_CLOSED:
+        /* Fall Through. */
+    case SCTP_CONNTRACK_NONE:
+        /* Fall Through. */
+    default:
+        return CT_DPIF_SCTP_STATE_CLOSED;
+    }
+#endif
+}
+
+static bool
+nl_ct_parse_protoinfo_sctp(struct nlattr *nla,
+                           struct ct_dpif_protoinfo *protoinfo)
+{
+    static const struct nl_policy policy[] = {
+        [CTA_PROTOINFO_SCTP_STATE] = { .type = NL_A_U8, .optional = false },
+        [CTA_PROTOINFO_SCTP_VTAG_ORIGINAL] = { .type = NL_A_U32,
+                                               .optional = false },
+        [CTA_PROTOINFO_SCTP_VTAG_REPLY] = { .type = NL_A_U32,
+                                            .optional = false },
+    };
+    struct nlattr *attrs[ARRAY_SIZE(policy)];
+    bool parsed;
+
+    parsed = nl_parse_nested(nla, policy, attrs, ARRAY_SIZE(policy));
+    if (parsed) {
+        protoinfo->proto = IPPROTO_SCTP;
+
+        protoinfo->sctp.state = nl_ct_sctp_state_to_dpif(
+            nl_attr_get_u8(attrs[CTA_PROTOINFO_SCTP_STATE]));
+        protoinfo->sctp.vtag_orig = nl_attr_get_u32(
+            attrs[CTA_PROTOINFO_SCTP_VTAG_ORIGINAL]);
+        protoinfo->sctp.vtag_reply = nl_attr_get_u32(
+            attrs[CTA_PROTOINFO_SCTP_VTAG_REPLY]);
+    } else {
+        VLOG_ERR_RL(&rl, "Could not parse nested SCTP protoinfo options. "
+                    "Possibly incompatible Linux kernel version.");
+    }
+
+    return parsed;
+}
+
 static bool
 nl_ct_parse_protoinfo(struct nlattr *nla, struct ct_dpif_protoinfo *protoinfo)
 {
@@ -737,7 +806,8 @@  nl_ct_parse_protoinfo(struct nlattr *nla, struct ct_dpif_protoinfo *protoinfo)
             parsed = nl_ct_parse_protoinfo_tcp(attrs[CTA_PROTOINFO_TCP],
                                                protoinfo);
         } else if (attrs[CTA_PROTOINFO_SCTP]) {
-            VLOG_WARN_RL(&rl, "SCTP protoinfo not yet supported!");
+            parsed = nl_ct_parse_protoinfo_sctp(attrs[CTA_PROTOINFO_SCTP],
+                                                protoinfo);
         } else {
             VLOG_WARN_RL(&rl, "Empty protoinfo!");
         }