[ovs-dev] [PATCH v3 1/3] dpif: Add support for OVS_ACTION_ATTR_CT_CLEAR
Flavio Leitner
fbl at sysclose.org
Mon Nov 20 13:59:10 UTC 2017
On Sat, 18 Nov 2017 19:41:44 -0500
Eric Garver <e at erig.me> wrote:
> This supports using the ct_clear action in the kernel datapath. To
> preserve compatibility with current ct_clear behavior on old kernels, we
> only pass this action down to the datapath if a probe reveals the
> datapath actually supports it.
>
> Signed-off-by: Eric Garver <e at erig.me>
> Acked-by: William Tu <u9012063 at gmail.com>
> ---
> NEWS | 3 +++
> datapath/linux/compat/include/linux/openvswitch.h | 2 ++
> lib/conntrack.c | 10 +++++++
> lib/conntrack.h | 1 +
> lib/dpif-netdev.c | 1 +
> lib/dpif.c | 1 +
> lib/odp-execute.c | 7 +++++
> lib/odp-util.c | 11 ++++++++
> lib/ofp-actions.c | 1 +
> ofproto/ofproto-dpif-ipfix.c | 1 +
> ofproto/ofproto-dpif-sflow.c | 1 +
> ofproto/ofproto-dpif-xlate.c | 14 +++++++++-
> ofproto/ofproto-dpif.c | 32 +++++++++++++++++++++++
> ofproto/ofproto-dpif.h | 5 +++-
> tests/odp.at | 1 +
> 15 files changed, 89 insertions(+), 2 deletions(-)
>
> diff --git a/NEWS b/NEWS
> index a93237f9078b..41123aab682e 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -12,6 +12,9 @@ Post-v2.8.0
> IPv6 packets.
> - Linux kernel 4.13
> * Add support for compiling OVS with the latest Linux 4.13 kernel
> + - OpenFlow:
> + * ct_clear action is now backed by kernel datapath. Support is probed for
> + when OVS starts.
>
> v2.8.0 - 31 Aug 2017
> --------------------
> diff --git a/datapath/linux/compat/include/linux/openvswitch.h b/datapath/linux/compat/include/linux/openvswitch.h
> index 561f8950277d..4874d5065e0a 100644
> --- a/datapath/linux/compat/include/linux/openvswitch.h
> +++ b/datapath/linux/compat/include/linux/openvswitch.h
> @@ -887,6 +887,7 @@ enum ovs_nat_attr {
> * @OVS_ACTION_ATTR_PUSH_ETH: Push a new outermost Ethernet header onto the
> * packet.
> * @OVS_ACTION_ATTR_POP_ETH: Pop the outermost Ethernet header off the packet.
> + * @OVS_ACTION_ATTR_CT_CLEAR: Clear conntrack state from the packet.
> * @OVS_ACTION_ATTR_ENCAP_NSH: encap NSH action to push NSH header.
> * @OVS_ACTION_ATTR_DECAP_NSH: decap NSH action to remove NSH header.
> *
> @@ -924,6 +925,7 @@ enum ovs_action_attr {
> OVS_ACTION_ATTR_TRUNC, /* u32 struct ovs_action_trunc. */
> OVS_ACTION_ATTR_PUSH_ETH, /* struct ovs_action_push_eth. */
> OVS_ACTION_ATTR_POP_ETH, /* No argument. */
> + OVS_ACTION_ATTR_CT_CLEAR, /* No argument. */
>
> #ifndef __KERNEL__
> OVS_ACTION_ATTR_TUNNEL_PUSH, /* struct ovs_action_push_tnl*/
> diff --git a/lib/conntrack.c b/lib/conntrack.c
> index f5a3aa9fab34..307902cea413 100644
> --- a/lib/conntrack.c
> +++ b/lib/conntrack.c
> @@ -1242,6 +1242,16 @@ conntrack_execute(struct conntrack *ct, struct dp_packet_batch *pkt_batch,
> return 0;
> }
>
> +int
> +conntrack_clear(struct dp_packet *packet)
> +{
> + /* According to pkt_metadata_init(), ct_state == 0 is enough to make all of
> + * the conntrack fields invalid. */
> + packet->md.ct_state = 0;
> +
> + return 0;
> +}
> +
> static void
> set_mark(struct dp_packet *pkt, struct conn *conn, uint32_t val, uint32_t mask)
> {
> diff --git a/lib/conntrack.h b/lib/conntrack.h
> index fbeef1c4754e..6c19f3c65804 100644
> --- a/lib/conntrack.h
> +++ b/lib/conntrack.h
> @@ -97,6 +97,7 @@ int conntrack_execute(struct conntrack *, struct dp_packet_batch *,
> const char *helper,
> const struct nat_action_info_t *nat_action_info,
> long long now);
> +int conntrack_clear(struct dp_packet *packet);
>
> struct conntrack_dump {
> struct conntrack *ct;
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index db7831874fed..e017368d6dae 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -5660,6 +5660,7 @@ dp_execute_cb(void *aux_, struct dp_packet_batch *packets_,
> case OVS_ACTION_ATTR_CLONE:
> case OVS_ACTION_ATTR_ENCAP_NSH:
> case OVS_ACTION_ATTR_DECAP_NSH:
> + case OVS_ACTION_ATTR_CT_CLEAR:
> case __OVS_ACTION_ATTR_MAX:
> OVS_NOT_REACHED();
> }
> diff --git a/lib/dpif.c b/lib/dpif.c
> index 3f0742d382ed..7bc4f9b3dc32 100644
> --- a/lib/dpif.c
> +++ b/lib/dpif.c
> @@ -1275,6 +1275,7 @@ dpif_execute_helper_cb(void *aux_, struct dp_packet_batch *packets_,
> case OVS_ACTION_ATTR_CLONE:
> case OVS_ACTION_ATTR_ENCAP_NSH:
> case OVS_ACTION_ATTR_DECAP_NSH:
> + case OVS_ACTION_ATTR_CT_CLEAR:
> case OVS_ACTION_ATTR_UNSPEC:
> case __OVS_ACTION_ATTR_MAX:
> OVS_NOT_REACHED();
> diff --git a/lib/odp-execute.c b/lib/odp-execute.c
> index 30114795584e..9fd084c1d2e7 100644
> --- a/lib/odp-execute.c
> +++ b/lib/odp-execute.c
> @@ -34,6 +34,7 @@
> #include "unaligned.h"
> #include "util.h"
> #include "csum.h"
> +#include "conntrack.h"
>
> /* Masked copy of an ethernet address. 'src' is already properly masked. */
> static void
> @@ -655,6 +656,7 @@ requires_datapath_assistance(const struct nlattr *a)
> case OVS_ACTION_ATTR_CLONE:
> case OVS_ACTION_ATTR_ENCAP_NSH:
> case OVS_ACTION_ATTR_DECAP_NSH:
> + case OVS_ACTION_ATTR_CT_CLEAR:
> return false;
>
> case OVS_ACTION_ATTR_UNSPEC:
> @@ -839,6 +841,11 @@ odp_execute_actions(void *dp, struct dp_packet_batch *batch, bool steal,
> }
> break;
> }
> + case OVS_ACTION_ATTR_CT_CLEAR:
> + DP_PACKET_BATCH_FOR_EACH (packet, batch) {
> + conntrack_clear(packet);
> + }
> + break;
>
> case OVS_ACTION_ATTR_OUTPUT:
> case OVS_ACTION_ATTR_TUNNEL_PUSH:
> diff --git a/lib/odp-util.c b/lib/odp-util.c
> index b348ab680d9f..d989f9e1d9fb 100644
> --- a/lib/odp-util.c
> +++ b/lib/odp-util.c
> @@ -117,6 +117,7 @@ odp_action_len(uint16_t type)
> case OVS_ACTION_ATTR_SET_MASKED: return ATTR_LEN_VARIABLE;
> case OVS_ACTION_ATTR_SAMPLE: return ATTR_LEN_VARIABLE;
> case OVS_ACTION_ATTR_CT: return ATTR_LEN_VARIABLE;
> + case OVS_ACTION_ATTR_CT_CLEAR: return 0;
> case OVS_ACTION_ATTR_PUSH_ETH: return sizeof(struct ovs_action_push_eth);
> case OVS_ACTION_ATTR_POP_ETH: return 0;
> case OVS_ACTION_ATTR_CLONE: return ATTR_LEN_VARIABLE;
> @@ -1045,6 +1046,9 @@ format_odp_action(struct ds *ds, const struct nlattr *a,
> case OVS_ACTION_ATTR_CT:
> format_odp_conntrack_action(ds, a);
> break;
> + case OVS_ACTION_ATTR_CT_CLEAR:
> + ds_put_cstr(ds, "ct_clear");
> + break;
> case OVS_ACTION_ATTR_CLONE:
> format_odp_clone_action(ds, a, portno_names);
> break;
> @@ -2122,6 +2126,13 @@ parse_odp_action(const char *s, const struct simap *port_names,
> }
>
> {
> + if (!strncmp(s, "ct_clear", 8)) {
> + nl_msg_put_flag(actions, OVS_ACTION_ATTR_CT_CLEAR);
> + return 8;
> + }
> + }
> +
> + {
> int retval;
>
> retval = parse_conntrack_action(s, actions);
> diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
> index e5d03e155543..a8f861e513f9 100644
> --- a/lib/ofp-actions.c
> +++ b/lib/ofp-actions.c
> @@ -7315,6 +7315,7 @@ ofpacts_execute_action_set(struct ofpbuf *action_list,
> if (!ofpacts_copy_last(action_list, action_set, OFPACT_GROUP) &&
> !ofpacts_copy_last(action_list, action_set, OFPACT_OUTPUT) &&
> !ofpacts_copy_last(action_list, action_set, OFPACT_RESUBMIT) &&
> + !ofpacts_copy_last(action_list, action_set, OFPACT_CT_CLEAR) &&
> !ofpacts_copy_last(action_list, action_set, OFPACT_CT)) {
> ofpbuf_clear(action_list);
> }
> diff --git a/ofproto/ofproto-dpif-ipfix.c b/ofproto/ofproto-dpif-ipfix.c
> index 4d168784beb9..4fba247dd816 100644
> --- a/ofproto/ofproto-dpif-ipfix.c
> +++ b/ofproto/ofproto-dpif-ipfix.c
> @@ -2813,6 +2813,7 @@ dpif_ipfix_read_actions(const struct flow *flow,
> case OVS_ACTION_ATTR_TRUNC:
> case OVS_ACTION_ATTR_HASH:
> case OVS_ACTION_ATTR_CT:
> + case OVS_ACTION_ATTR_CT_CLEAR:
> case OVS_ACTION_ATTR_METER:
> case OVS_ACTION_ATTR_SET_MASKED:
> case OVS_ACTION_ATTR_SET:
> diff --git a/ofproto/ofproto-dpif-sflow.c b/ofproto/ofproto-dpif-sflow.c
> index ccf89645bd66..039ac4c4d892 100644
> --- a/ofproto/ofproto-dpif-sflow.c
> +++ b/ofproto/ofproto-dpif-sflow.c
> @@ -1160,6 +1160,7 @@ dpif_sflow_read_actions(const struct flow *flow,
> case OVS_ACTION_ATTR_RECIRC:
> case OVS_ACTION_ATTR_HASH:
> case OVS_ACTION_ATTR_CT:
> + case OVS_ACTION_ATTR_CT_CLEAR:
> case OVS_ACTION_ATTR_METER:
> break;
>
> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
> index 468cd160c60e..6d6fcaaca453 100644
> --- a/ofproto/ofproto-dpif-xlate.c
> +++ b/ofproto/ofproto-dpif-xlate.c
> @@ -4382,6 +4382,7 @@ xlate_fixup_actions(struct ofpbuf *b, const struct nlattr *actions,
> case OVS_ACTION_ATTR_USERSPACE:
> case OVS_ACTION_ATTR_RECIRC:
> case OVS_ACTION_ATTR_CT:
> + case OVS_ACTION_ATTR_CT_CLEAR:
> case OVS_ACTION_ATTR_PUSH_ETH:
> case OVS_ACTION_ATTR_POP_ETH:
> case OVS_ACTION_ATTR_ENCAP_NSH:
> @@ -5795,6 +5796,17 @@ compose_conntrack_action(struct xlate_ctx *ctx, struct ofpact_conntrack *ofc,
> }
>
> static void
> +compose_ct_clear_action(struct xlate_ctx *ctx)
> +{
> + clear_conntrack(ctx);
> + /* This action originally existed without dpif support. So to preserve
> + * compatibility, only append it if the dpif supports it. */
> + if (ctx->xbridge->support.ct_clear) {
> + nl_msg_put_flag(ctx->odp_actions, OVS_ACTION_ATTR_CT_CLEAR);
> + }
> +}
> +
> +static void
> rewrite_flow_encap_ethernet(struct xlate_ctx *ctx,
> struct flow *flow,
> struct flow_wildcards *wc)
> @@ -6542,7 +6554,7 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len,
> break;
>
> case OFPACT_CT_CLEAR:
> - clear_conntrack(ctx);
> + compose_ct_clear_action(ctx);
> break;
>
> case OFPACT_NAT:
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index b14b339a962a..de0cf78b8f3b 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -1322,6 +1322,37 @@ check_ct_eventmask(struct dpif_backer *backer)
> return !error;
> }
>
> +static bool
> +check_ct_clear(struct dpif_backer *backer)
> +{
> + struct odputil_keybuf keybuf;
> + uint8_t actbuf[NL_A_FLAG_SIZE];
> + struct ofpbuf actions;
> + struct ofpbuf key;
> + struct flow flow;
> + bool supported;
> +
> + struct odp_flow_key_parms odp_parms = {
> + .flow = &flow,
> + .probe = true,
> + };
> +
> + memset(&flow, 0, sizeof flow);
> + ofpbuf_use_stack(&key, &keybuf, sizeof keybuf);
> + odp_flow_key_from_flow(&odp_parms, &key);
> +
> + ofpbuf_use_stack(&actions, &actbuf, sizeof actbuf);
> + nl_msg_put_flag(&actions, OVS_ACTION_ATTR_CT_CLEAR);
> +
> + supported = dpif_probe_feature(backer->dpif, "ct_clear", &key,
> + &actions, NULL);
> +
> + VLOG_INFO("%s: Datapath %s ct_clear action",
> + dpif_name(backer->dpif), (supported) ? "supports"
> + : "does not support");
> + return supported;
> +}
> +
> #define CHECK_FEATURE__(NAME, SUPPORT, FIELD, VALUE, ETHTYPE) \
> static bool \
> check_##NAME(struct dpif_backer *backer) \
> @@ -1386,6 +1417,7 @@ check_support(struct dpif_backer *backer)
> backer->rt_support.clone = check_clone(backer);
> backer->rt_support.sample_nesting = check_max_sample_nesting(backer);
> backer->rt_support.ct_eventmask = check_ct_eventmask(backer);
> + backer->rt_support.ct_clear = check_ct_clear(backer);
>
> /* Flow fields. */
> backer->rt_support.odp.ct_state = check_ct_state(backer);
> diff --git a/ofproto/ofproto-dpif.h b/ofproto/ofproto-dpif.h
> index 0857c070c8ac..8752061d6439 100644
> --- a/ofproto/ofproto-dpif.h
> +++ b/ofproto/ofproto-dpif.h
> @@ -178,7 +178,10 @@ struct group_dpif *group_dpif_lookup(struct ofproto_dpif *,
> DPIF_SUPPORT_FIELD(size_t, sample_nesting, "Sample nesting") \
> \
> /* OVS_CT_ATTR_EVENTMASK supported by OVS_ACTION_ATTR_CT action. */ \
> - DPIF_SUPPORT_FIELD(bool, ct_eventmask, "Conntrack eventmask")
> + DPIF_SUPPORT_FIELD(bool, ct_eventmask, "Conntrack eventmask") \
> + \
> + /* True if the datapath supports OVS_ACTION_ATTR_CT_CLEAR action. */ \
> + DPIF_SUPPORT_FIELD(bool, ct_clear, "Conntrack clear")
>
> /* Stores the various features which the corresponding backer supports. */
> struct dpif_backer_support {
> diff --git a/tests/odp.at b/tests/odp.at
> index cd01b32d72ef..f1ab30727ddb 100644
> --- a/tests/odp.at
> +++ b/tests/odp.at
> @@ -354,6 +354,7 @@ ct(force_commit,nat(src=fe80::20c:29ff:fe88:a18b,random))
> ct(force_commit,nat(src=fe80::20c:29ff:fe88:1-fe80::20c:29ff:fe88:a18b,random))
> ct(force_commit,nat(src=[[fe80::20c:29ff:fe88:1]]-[[fe80::20c:29ff:fe88:a18b]]:255-4096,random))
> ct(force_commit,helper=ftp,nat(src=10.1.1.240-10.1.1.255))
> +ct_clear
> trunc(100)
> clone(1)
> clone(clone(push_vlan(vid=12,pcp=0),2),1)
Acked-by: Flavio Leitner <fbl at sysclose.org>
Thanks!
--
Flavio
More information about the dev
mailing list