datapath: Do not trigger events for unconfirmed connections.

Upstream commit:

    commit 193e30967897f3a8b6f9f137ac30571d832c2c5c
    Author: Jarno Rajahalme <jarno@ovn.org>
    Date:   Thu Feb 9 11:21:54 2017 -0800

    openvswitch: Do not trigger events for unconfirmed connections.
    Receiving change events before the 'new' event for the connection has
    been received can be confusing.  Avoid triggering change events for
    setting conntrack mark or labels before the conntrack entry has been
    confirmed.

    Fixes: 182e3042e15d ("openvswitch: Allow matching on conntrack mark")
    Fixes: c2ac66735870 ("openvswitch: Allow matching on conntrack label")
    Signed-off-by: Jarno Rajahalme <jarno@ovn.org>
    Acked-by: Joe Stringer <joe@ovn.org>
    Acked-by: Pravin B Shelar <pshelar@ovn.org>
    Signed-off-by: David S. Miller <davem@davemloft.net>

Upstream commit:

    commit 2317c6b51e4249dbfa093e1b88cab0a9f0564b7f
    Author: Jarno Rajahalme <jarno@ovn.org>
    Date:   Fri Feb 17 18:11:58 2017 -0800

    openvswitch: Set event bit after initializing labels.

    Connlabels are included in conntrack netlink event messages only if
    the IPCT_LABEL bit is set in the event cache (see
    ctnetlink_conntrack_event()).  Set it after initializing labels for a
    new connection.

    Found upon further system testing, where it was noticed that labels
    were missing from the conntrack events.

    Fixes: 193e30967897 ("openvswitch: Do not trigger events for unconfirmed con
nections.")
    Signed-off-by: Jarno Rajahalme <jarno@ovn.org>
    Acked-by: Pravin B Shelar <pshelar@ovn.org>
    Signed-off-by: David S. Miller <davem@davemloft.net>

Fixes: 372ce9737d2b ("datapath: Allow matching on conntrack mark")
Fixes: 038e34abaa31 ("datapath: Allow matching on conntrack label")
Signed-off-by: Jarno Rajahalme <jarno@ovn.org>

1 files changed, 27 insertions, 6 deletions

diff --git a/datapath/conntrack.c b/datapath/conntrack.c
index 10a7b911d..9595fca84 100644
--- a/datapath/conntrack.c
+++ b/datapath/conntrack.c
@@ -261,7 +261,8 @@ static int ovs_ct_set_mark(struct sk_buff *skb, struct sw_flow_key *key,
 	new_mark = ct_mark | (ct->mark & ~(mask));
 	if (ct->mark != new_mark) {
 		ct->mark = new_mark;
-		nf_conntrack_event_cache(IPCT_MARK, ct);
+		if (nf_ct_is_confirmed(ct))
+			nf_conntrack_event_cache(IPCT_MARK, ct);
 		key->ct.mark = new_mark;
 	}
 
@@ -278,7 +279,6 @@ static int ovs_ct_set_labels(struct sk_buff *skb, struct sw_flow_key *key,
 	enum ip_conntrack_info ctinfo;
 	struct nf_conn_labels *cl;
 	struct nf_conn *ct;
-	int err;
 
 	/* The connection could be invalid, in which case set_label is no-op.*/
 	ct = nf_ct_get(skb, &ctinfo);
@@ -294,10 +294,31 @@ static int ovs_ct_set_labels(struct sk_buff *skb, struct sw_flow_key *key,
 	if (!cl || ovs_ct_get_labels_len(cl) < OVS_CT_LABELS_LEN)
 		return -ENOSPC;
 
-	err = nf_connlabels_replace(ct, (u32 *)labels, (u32 *)mask,
-				    OVS_CT_LABELS_LEN / sizeof(u32));
-	if (err)
-		return err;
+	if (nf_ct_is_confirmed(ct)) {
+		/* Triggers a change event, which makes sense only for
+		 * confirmed connections.
+		 */
+		int err = nf_connlabels_replace(ct, (u32 *)labels, (u32 *)mask,
+						OVS_CT_LABELS_LEN / sizeof(u32));
+		if (err)
+			return err;
+	} else {
+		u32 *dst = (u32 *)cl->bits;
+		const u32 *msk = (const u32 *)mask->ct_labels;
+		const u32 *lbl = (const u32 *)labels->ct_labels;
+		int i;
+
+		/* No-one else has access to the non-confirmed entry, copy
+		 * labels over, keeping any bits we are not explicitly setting.
+		 */
+		for (i = 0; i < OVS_CT_LABELS_LEN / sizeof(u32); i++)
+			dst[i] = (dst[i] & ~msk[i]) | (lbl[i] & msk[i]);
+
+		/* Labels are included in the IPCTNL_MSG_CT_NEW event only if
+		 * the IPCT_LABEL bit it set in the event cache.
+		 */
+		nf_conntrack_event_cache(IPCT_LABEL, ct);
+	}
 
 	ovs_ct_get_labels(ct, &key->ct.labels);
 	return 0;