Re: [ovs-dev] [PATCH v3] xlate: Use OVS_CT_ATTR_EVENTMASK.

[Jarno Rajahalme]

> On Apr 27, 2017, at 4:15 PM, Joe Stringer  wrote:
> 
> On 27 April 2017 at 14:48, Jarno Rajahalme  > wrote:
>> Specify the event mask with CT commit including bits for CT features
>> exposed at the OVS interface (mark and label changes in addition to
>> basic creation and destruction of conntrack entries).
>> 
>> Without this any listener of conntrack update events will typically
>> (depending on system configuration) receive events for each L4 (e.g.,
>> TCP) state machine change, which can multiply the number of events
>> received per connection.
>> 
>> By including the new, related, and destroy events any listener of new
>> conntrack events gets notified of new related and non-related
>> connections, and any listener of destroy events will get notified of
>> deleted (typically timed out) conntrack entries.
>> 
>> By including the flags for mark and labels, any listener of conntrack
>> update events gets notified whenever the connmark or conntrack labels
>> are changed from the values reported within the new events.
>> 
>> VMware-BZ: #1837218
>> Signed-off-by: Jarno Rajahalme 
>> Acked-by: Joe Stringer 
>> ---
> 
> Did you try building with sparse? I'm seeing:
> 
> ofproto/ofproto-dpif.c:1256:19: error: incorrect type in initializer
> (different base types)
> ofproto/ofproto-dpif.c:1256:19: expected restricted ovs_be32 [usertype] nw_src
> ofproto/ofproto-dpif.c:1256:19: got int
> ofproto/ofproto-dpif.c:1257:19: error: incorrect type in initializer
> (different base types)
> ofproto/ofproto-dpif.c:1257:19: expected restricted ovs_be32 [usertype] nw_dst
> ofproto/ofproto-dpif.c:1257:19: got int
> ofproto/ofproto-dpif.c:1258:19: error: incorrect type in initializer
> (different base types)
> ofproto/ofproto-dpif.c:1258:19: expected restricted ovs_be16 [usertype] tp_src
> ofproto/ofproto-dpif.c:1258:19: got int
> ofproto/ofproto-dpif.c:1259:19: error: incorrect type in initializer
> (different base types)
> ofproto/ofproto-dpif.c:1259:19: expected restricted ovs_be16 [usertype] tp_dst
> ofproto/ofproto-dpif.c:1259:19: got int

Thanks for reporting this! I’ve been unlucky getting sparse to not give me a 
ton of meaningless errors…

Posted a v4 fixing this,

  Jarno

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v3] xlate: Use OVS_CT_ATTR_EVENTMASK.

[Joe Stringer]

On 27 April 2017 at 14:48, Jarno Rajahalme  wrote:
> Specify the event mask with CT commit including bits for CT features
> exposed at the OVS interface (mark and label changes in addition to
> basic creation and destruction of conntrack entries).
>
> Without this any listener of conntrack update events will typically
> (depending on system configuration) receive events for each L4 (e.g.,
> TCP) state machine change, which can multiply the number of events
> received per connection.
>
> By including the new, related, and destroy events any listener of new
> conntrack events gets notified of new related and non-related
> connections, and any listener of destroy events will get notified of
> deleted (typically timed out) conntrack entries.
>
> By including the flags for mark and labels, any listener of conntrack
> update events gets notified whenever the connmark or conntrack labels
> are changed from the values reported within the new events.
>
> VMware-BZ: #1837218
> Signed-off-by: Jarno Rajahalme 
> Acked-by: Joe Stringer 
> ---

Did you try building with sparse? I'm seeing:

ofproto/ofproto-dpif.c:1256:19: error: incorrect type in initializer
(different base types)
ofproto/ofproto-dpif.c:1256:19: expected restricted ovs_be32 [usertype] nw_src
ofproto/ofproto-dpif.c:1256:19: got int
ofproto/ofproto-dpif.c:1257:19: error: incorrect type in initializer
(different base types)
ofproto/ofproto-dpif.c:1257:19: expected restricted ovs_be32 [usertype] nw_dst
ofproto/ofproto-dpif.c:1257:19: got int
ofproto/ofproto-dpif.c:1258:19: error: incorrect type in initializer
(different base types)
ofproto/ofproto-dpif.c:1258:19: expected restricted ovs_be16 [usertype] tp_src
ofproto/ofproto-dpif.c:1258:19: got int
ofproto/ofproto-dpif.c:1259:19: error: incorrect type in initializer
(different base types)
ofproto/ofproto-dpif.c:1259:19: expected restricted ovs_be16 [usertype] tp_dst
ofproto/ofproto-dpif.c:1259:19: got int
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v3] xlate: Use OVS_CT_ATTR_EVENTMASK.

[Jarno Rajahalme]

Specify the event mask with CT commit including bits for CT features
exposed at the OVS interface (mark and label changes in addition to
basic creation and destruction of conntrack entries).

Without this any listener of conntrack update events will typically
(depending on system configuration) receive events for each L4 (e.g.,
TCP) state machine change, which can multiply the number of events
received per connection.

By including the new, related, and destroy events any listener of new
conntrack events gets notified of new related and non-related
connections, and any listener of destroy events will get notified of
deleted (typically timed out) conntrack entries.

By including the flags for mark and labels, any listener of conntrack
update events gets notified whenever the connmark or conntrack labels
are changed from the values reported within the new events.

VMware-BZ: #1837218
Signed-off-by: Jarno Rajahalme 
Acked-by: Joe Stringer 
---
v3: Added feature probing to not use the new attribute on datapaths
the do not support it.

 build-aux/extract-odp-netlink-h |  2 ++
 ofproto/ofproto-dpif-xlate.c|  6 
 ofproto/ofproto-dpif.c  | 62 +
 ofproto/ofproto-dpif.h  |  5 +++-
 4 files changed, 74 insertions(+), 1 deletion(-)

diff --git a/build-aux/extract-odp-netlink-h b/build-aux/extract-odp-netlink-h
index 907a70a..7fb6ce8 100755
--- a/build-aux/extract-odp-netlink-h
+++ b/build-aux/extract-odp-netlink-h
@@ -19,6 +19,8 @@ $i\
 #ifdef _WIN32\
 #include "OvsDpInterfaceExt.h"\
 #include "OvsDpInterfaceCtExt.h"\
+#else\
+#include "linux/netfilter/nf_conntrack_common.h"\
 #endif\
 
 # Use OVS's own struct eth_addr instead of a 6-byte char array.
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index d8c6a7c..ab5eef8 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -5351,6 +5351,12 @@ compose_conntrack_action(struct xlate_ctx *ctx, struct 
ofpact_conntrack *ofc)
 if (ofc->flags & NX_CT_F_COMMIT) {
 nl_msg_put_flag(ctx->odp_actions, ofc->flags & NX_CT_F_FORCE ?
 OVS_CT_ATTR_FORCE_COMMIT : OVS_CT_ATTR_COMMIT);
+if (ctx->xbridge->support.ct_eventmask) {
+nl_msg_put_u32(ctx->odp_actions, OVS_CT_ATTR_EVENTMASK,
+   1 << IPCT_NEW | 1 << IPCT_RELATED |
+   1 << IPCT_DESTROY | 1 << IPCT_MARK |
+   1 << IPCT_LABEL);
+}
 }
 nl_msg_put_u16(ctx->odp_actions, OVS_CT_ATTR_ZONE, zone);
 put_ct_mark(>xin->flow, ctx->odp_actions, ctx->wc);
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index c73c273..b052b04 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -1241,6 +1241,67 @@ check_clone(struct dpif_backer *backer)
 return !error;
 }
 
+/* Tests whether 'backer''s datapath supports the OVS_CT_ATTR_EVENTMASK
+ * attribute in OVS_ACTION_ATTR_CT. */
+static bool
+check_ct_eventmask(struct dpif_backer *backer)
+{
+struct dpif_execute execute;
+struct dp_packet packet;
+struct ofpbuf actions;
+struct flow flow = {
+.dl_type = htons(ETH_TYPE_IP),
+.nw_ttl = 64,
+.nw_proto = IPPROTO_UDP,
+.nw_src = 0x0a010101,
+.nw_dst = 0x0a010102,
+.tp_src = 42387,
+.tp_dst = 13264,
+};
+size_t ct_start;
+int error;
+
+/* Compose CT action with eventmask attribute and check if datapath can
+ * decode the message.  */
+ofpbuf_init(, 64);
+ct_start = nl_msg_start_nested(, OVS_ACTION_ATTR_CT);
+/* Eventmask has no effect without the commit flag, but currently the
+ * datapath will accept an eventmask even without commit.  This is useful
+ * as we do not want to persist the probe connection in the conntrack
+ * table. */
+nl_msg_put_u32(, OVS_CT_ATTR_EVENTMASK, ~0);
+nl_msg_end_nested(, ct_start);
+
+/* Compose a dummy UDP packet. */
+dp_packet_init(, 0);
+flow_compose(, );
+
+/* Execute the actions.  On older datapaths this fails with EINVAL, on
+ * newer datapaths it succeeds. */
+execute.actions = actions.data;
+execute.actions_len = actions.size;
+execute.packet = 
+execute.flow = 
+execute.needs_help = false;
+execute.probe = true;
+execute.mtu = 0;
+
+error = dpif_execute(backer->dpif, );
+
+dp_packet_uninit();
+ofpbuf_uninit();
+
+if (error) {
+VLOG_INFO("%s: Datapath does not support eventmask in conntrack 
action",
+  dpif_name(backer->dpif));
+} else {
+VLOG_INFO("%s: Datapath supports eventmask in conntrack action",
+  dpif_name(backer->dpif));
+}
+
+return !error;
+}
+
 #define CHECK_FEATURE__(NAME, SUPPORT, FIELD, VALUE)\
 static bool \
 check_##NAME(struct dpif_backer *backer)