[ovs-dev,branch-2.7,21/25] datapath: Inherit master's labels.

Message ID 1489620689-122370-22-git-send-email-jarno@ovn.org
State Superseded
Headers show

Commit Message

Jarno Rajahalme March 15, 2017, 11:31 p.m.
Upstream commit:

    commit 09aa98ad496d6b11a698b258bc64d7f64c55d682
    Author: Jarno Rajahalme <jarno@ovn.org>
    Date:   Thu Feb 9 11:21:58 2017 -0800

    openvswitch: Inherit master's labels.

    We avoid calling into nf_conntrack_in() for expected connections, as
    that would remove the expectation that we want to stick around until
    we are ready to commit the connection.  Instead, we do a lookup in the
    expectation table directly.  However, after a successful expectation
    lookup we have set the flow key label field from the master
    connection, whereas nf_conntrack_in() does not do this.  This leads to
    master's labels being inherited after an expectation lookup, but those
    labels not being inherited after the corresponding conntrack action
    with a commit flag.

    This patch resolves the problem by changing the commit code path to
    also inherit the master's labels to the expected connection.
    Resolving this conflict in favor of inheriting the labels allows more
    information be passed from the master connection to related
    connections, which would otherwise be much harder if the 32 bits in
    the connmark are not enough.  Labels can still be set explicitly, so
    this change only affects the default values of the labels in presense
    of a master connection.

    Fixes: 7f8a436eaa2c ("openvswitch: Add conntrack action")
    Signed-off-by: Jarno Rajahalme <jarno@ovn.org>
    Acked-by: Pravin B Shelar <pshelar@ovn.org>
    Acked-by: Joe Stringer <joe@ovn.org>
    Signed-off-by: David S. Miller <davem@davemloft.net>

Fixes: a94ebc39996b ("datapath: Add conntrack action")
Signed-off-by: Jarno Rajahalme <jarno@ovn.org>
Acked-by: Joe Stringer <joe@ovn.org>
---
 datapath/conntrack.c | 45 +++++++++++++++++++++++++++++++--------------
 1 file changed, 31 insertions(+), 14 deletions(-)

Patch

diff --git a/datapath/conntrack.c b/datapath/conntrack.c
index a56fe07..9428eb2 100644
--- a/datapath/conntrack.c
+++ b/datapath/conntrack.c
@@ -80,6 +80,8 @@  struct ovs_conntrack_info {
 #endif
 };
 
+static bool labels_nonzero(const struct ovs_key_ct_labels *labels);
+
 static void __ovs_ct_free_action(struct ovs_conntrack_info *ct_info);
 
 static u16 key_to_nfproto(const struct sw_flow_key *key)
@@ -275,18 +277,32 @@  static int ovs_ct_init_labels(struct nf_conn *ct, struct sw_flow_key *key,
 			      const struct ovs_key_ct_labels *labels,
 			      const struct ovs_key_ct_labels *mask)
 {
-	struct nf_conn_labels *cl;
-	u32 *dst;
-	int i;
+	struct nf_conn_labels *cl, *master_cl;
+	bool have_mask = labels_nonzero(mask);
+
+	/* Inherit master's labels to the related connection? */
+	master_cl = ct->master ? nf_ct_labels_find(ct->master) : NULL;
+
+	if (!master_cl && !have_mask)
+		return 0;   /* Nothing to do. */
 
 	cl = ovs_ct_get_conn_labels(ct);
 	if (!cl)
 		return -ENOSPC;
 
-	dst = (u32 *)cl->bits;
-	for (i = 0; i < OVS_CT_LABELS_LEN_32; i++)
-		dst[i] = (dst[i] & ~mask->ct_labels_32[i]) |
-			(labels->ct_labels_32[i] & mask->ct_labels_32[i]);
+	/* Inherit the master's labels, if any. */
+	if (master_cl)
+		*cl = *master_cl;
+
+	if (have_mask) {
+		u32 *dst = (u32 *)cl->bits;
+		int i;
+
+		for (i = 0; i < OVS_CT_LABELS_LEN_32; i++)
+			dst[i] = (dst[i] & ~mask->ct_labels_32[i]) |
+				(labels->ct_labels_32[i]
+				 & mask->ct_labels_32[i]);
+	}
 
 	/* Labels are included in the IPCTNL_MSG_CT_NEW event only if the
 	 * IPCT_LABEL bit it set in the event cache.
@@ -943,13 +959,14 @@  static int ovs_ct_commit(struct net *net, struct sw_flow_key *key,
 		if (err)
 			return err;
 	}
-	if (labels_nonzero(&info->labels.mask)) {
-		if (!nf_ct_is_confirmed(ct))
-			err = ovs_ct_init_labels(ct, key, &info->labels.value,
-						 &info->labels.mask);
-		else
-			err = ovs_ct_set_labels(ct, key, &info->labels.value,
-						&info->labels.mask);
+	if (!nf_ct_is_confirmed(ct)) {
+		err = ovs_ct_init_labels(ct, key, &info->labels.value,
+					 &info->labels.mask);
+		if (err)
+			return err;
+	} else if (labels_nonzero(&info->labels.mask)) {
+		err = ovs_ct_set_labels(ct, key, &info->labels.value,
+					&info->labels.mask);
 		if (err)
 			return err;
 	}