On 28 February 2017 at 17:17, Jarno Rajahalme <[email protected]> wrote:
> Upstream patch:
>
> commit dd41d33f0b033885211a5d6f3ee19e73238aa9ee
> Author: Jarno Rajahalme <[email protected]>
> 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 <[email protected]>
> Acked-by: Pravin B Shelar <[email protected]>
> Acked-by: Joe Stringer <[email protected]>
> Signed-off-by: David S. Miller <[email protected]>
>
> Signed-off-by: Jarno Rajahalme <[email protected]>
> ---
With the following minor incremental I was able to compile this
commit, otherwise looks good. Note that the below doesn't implement
the force logic yet, which comes in the next patch:
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 30907b7f27cf..d082cccc06b4 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:
case OVS_CT_ATTR_COMMIT:
commit = true;
break;
> datapath/conntrack.c | 26
> +++++++++++++++++++++--
> datapath/linux/compat/include/linux/openvswitch.h | 5 +++++
> 2 files changed, 29 insertions(+), 2 deletions(-)
>
> diff --git a/datapath/conntrack.c b/datapath/conntrack.c
> index d8309c9..041a557 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;
> @@ -658,10 +659,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)))
> @@ -675,6 +679,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;
> }
> @@ -1259,6 +1275,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),
> @@ -1298,6 +1315,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;
> @@ -1528,7 +1548,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 23f8845..2fd0963 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
> };
>
> --
> 2.1.4
>
> _______________________________________________
> dev mailing list
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
