Re: [ovs-dev] [PATCH v2] conntrack: add display support for sctp

2019-05-21 Thread Aaron Conole
Ben Pfaff  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 
>
> 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.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH v2] conntrack: add display support for sctp

2019-05-20 Thread Ben Pfaff
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 

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.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] [PATCH v2] conntrack: add display support for sctp

2019-05-17 Thread Aaron Conole
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 
---
 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(-)

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 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;
+