[ovs-dev] [PATCH v3 19/22] datapath: Add force commit.

Jarno Rajahalme jarno at ovn.org
Tue Mar 7 00:22:47 UTC 2017


Upstream patch:

    commit dd41d33f0b033885211a5d6f3ee19e73238aa9ee
    Author: Jarno Rajahalme <jarno at ovn.org>
    Date:   Thu Feb 9 11:22:00 2017 -0800

    openvswitch: Add force commit.

    Stateful network admission policy may allow connections to one
    direction and reject connections initiated in the other direction.
    After policy change it is possible that for a new connection an
    overlapping conntrack entry already exists, where the original
    direction of the existing connection is opposed to the new
    connection's initial packet.

    Most importantly, conntrack state relating to the current packet gets
    the "reply" designation based on whether the original direction tuple
    or the reply direction tuple matched.  If this "directionality" is
    wrong w.r.t. to the stateful network admission policy it may happen
    that packets in neither direction are correctly admitted.

    This patch adds a new "force commit" option to the OVS conntrack
    action that checks the original direction of an existing conntrack
    entry.  If that direction is opposed to the current packet, the
    existing conntrack entry is deleted and a new one is subsequently
    created in the correct direction.

    Signed-off-by: Jarno Rajahalme <jarno at ovn.org>
    Acked-by: Pravin B Shelar <pshelar at ovn.org>
    Acked-by: Joe Stringer <joe at ovn.org>
    Signed-off-by: David S. Miller <davem at davemloft.net>

Signed-off-by: Jarno Rajahalme <jarno at ovn.org>
---
 datapath/conntrack.c                              | 26 +++++++++++++++++++++--
 datapath/linux/compat/include/linux/openvswitch.h |  5 +++++
 lib/dpif-netdev.c                                 |  2 ++
 3 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/datapath/conntrack.c b/datapath/conntrack.c
index 11ebefe..4df7352 100644
--- a/datapath/conntrack.c
+++ b/datapath/conntrack.c
@@ -72,6 +72,7 @@ struct ovs_conntrack_info {
 	u8 commit : 1;
 	u8 nat : 3;                 /* enum ovs_ct_nat */
 	u8 random_fully_compat : 1; /* bool */
+	u8 force : 1;
 	u16 family;
 	struct md_mark mark;
 	struct md_labels labels;
@@ -647,10 +648,13 @@ static bool skb_nfct_cached(struct net *net,
 	 */
 	if (!ct && key->ct.state & OVS_CS_F_TRACKED &&
 	    !(key->ct.state & OVS_CS_F_INVALID) &&
-	    key->ct.zone == info->zone.id)
+	    key->ct.zone == info->zone.id) {
 		ct = ovs_ct_find_existing(net, &info->zone, info->family, skb,
 					  !!(key->ct.state
 					     & OVS_CS_F_NAT_MASK));
+		if (ct)
+			nf_ct_get(skb, &ctinfo);
+	}
 	if (!ct)
 		return false;
 	if (!net_eq(net, read_pnet(&ct->ct_net)))
@@ -664,6 +668,18 @@ static bool skb_nfct_cached(struct net *net,
 		if (help && rcu_access_pointer(help->helper) != info->helper)
 			return false;
 	}
+	/* Force conntrack entry direction to the current packet? */
+	if (info->force && CTINFO2DIR(ctinfo) != IP_CT_DIR_ORIGINAL) {
+		/* Delete the conntrack entry if confirmed, else just release
+		 * the reference.
+		 */
+		if (nf_ct_is_confirmed(ct))
+			nf_ct_delete(ct, 0, 0);
+		else
+			nf_conntrack_put(&ct->ct_general);
+		nf_ct_set(skb, NULL, 0);
+		return false;
+	}
 
 	return true;
 }
@@ -1246,6 +1262,7 @@ static int parse_nat(const struct nlattr *attr,
 
 static const struct ovs_ct_len_tbl ovs_ct_attr_lens[OVS_CT_ATTR_MAX + 1] = {
 	[OVS_CT_ATTR_COMMIT]	= { .minlen = 0, .maxlen = 0 },
+	[OVS_CT_ATTR_FORCE_COMMIT]	= { .minlen = 0, .maxlen = 0 },
 	[OVS_CT_ATTR_ZONE]	= { .minlen = sizeof(u16),
 				    .maxlen = sizeof(u16) },
 	[OVS_CT_ATTR_MARK]	= { .minlen = sizeof(struct md_mark),
@@ -1285,6 +1302,9 @@ static int parse_ct(const struct nlattr *attr, struct ovs_conntrack_info *info,
 		}
 
 		switch (type) {
+		case OVS_CT_ATTR_FORCE_COMMIT:
+			info->force = true;
+			/* fall through. */
 		case OVS_CT_ATTR_COMMIT:
 			info->commit = true;
 			break;
@@ -1515,7 +1535,9 @@ int ovs_ct_action_to_attr(const struct ovs_conntrack_info *ct_info,
 	if (!start)
 		return -EMSGSIZE;
 
-	if (ct_info->commit && nla_put_flag(skb, OVS_CT_ATTR_COMMIT))
+	if (ct_info->commit && nla_put_flag(skb, ct_info->force
+					    ? OVS_CT_ATTR_FORCE_COMMIT
+					    : OVS_CT_ATTR_COMMIT))
 		return -EMSGSIZE;
 	if (IS_ENABLED(CONFIG_NF_CONNTRACK_ZONES) &&
 	    nla_put_u16(skb, OVS_CT_ATTR_ZONE, ct_info->zone.id))
diff --git a/datapath/linux/compat/include/linux/openvswitch.h b/datapath/linux/compat/include/linux/openvswitch.h
index f92d859..3112214 100644
--- a/datapath/linux/compat/include/linux/openvswitch.h
+++ b/datapath/linux/compat/include/linux/openvswitch.h
@@ -718,6 +718,10 @@ struct ovs_action_push_tnl {
  * mask. For each bit set in the mask, the corresponding bit in the value is
  * copied to the connection tracking label field in the connection.
  * @OVS_CT_ATTR_HELPER: variable length string defining conntrack ALG.
+ * @OVS_CT_ATTR_FORCE_COMMIT: Like %OVS_CT_ATTR_COMMIT, but instead of doing
+ * nothing if the connection is already committed will check that the current
+ * packet is in conntrack entry's original direction.  If directionality does
+ * not match, will delete the existing conntrack entry and create a new one.
  */
 enum ovs_ct_attr {
 	OVS_CT_ATTR_UNSPEC,
@@ -728,6 +732,7 @@ enum ovs_ct_attr {
 	OVS_CT_ATTR_HELPER,     /* netlink helper to assist detection of
 				   related connections. */
 	OVS_CT_ATTR_NAT,        /* Nested OVS_NAT_ATTR_* */
+	OVS_CT_ATTR_FORCE_COMMIT,  /* No argument */
 	__OVS_CT_ATTR_MAX
 };
 
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index ebc6644..42196b2 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -4746,6 +4746,8 @@ dp_execute_cb(void *aux_, struct dp_packet_batch *packets_,
             enum ovs_ct_attr sub_type = nl_attr_type(b);
 
             switch(sub_type) {
+            case OVS_CT_ATTR_FORCE_COMMIT:
+                /* Not implemented yet. */
             case OVS_CT_ATTR_COMMIT:
                 commit = true;
                 break;
-- 
2.1.4



More information about the dev mailing list