[ovs-dev] [PATCH v4] datapath: Add a new action dec_ttl
Ilya Maximets
i.maximets at ovn.org
Tue May 18 18:15:16 UTC 2021
On 5/18/21 4:44 PM, Eelco Chaudron wrote:
> Add support for the dec_ttl action. Instead of programming the datapath with
> a flow that matches the packet TTL and an IP set, use a single dec_ttl action.
>
> The old behavior is kept if the new action is not supported by the datapath.
>
> # ovs-ofctl dump-flows br0
> cookie=0x0, duration=12.538s, table=0, n_packets=4, n_bytes=392, ip actions=dec_ttl,NORMAL
> cookie=0x0, duration=12.536s, table=0, n_packets=4, n_bytes=168, actions=NORMAL
>
> # ping -c1 -t 20 192.168.0.2
> PING 192.168.0.2 (192.168.0.2) 56(84) bytes of data.
> IP (tos 0x0, ttl 19, id 45336, offset 0, flags [DF], proto ICMP (1), length 84)
> 192.168.0.1 > 192.168.0.2: ICMP echo request, id 8865, seq 1, length 64
>
> Linux netlink datapath support depends on upstream Linux commit:
> 744676e77720 ("openvswitch: add TTL decrement action")
>
>
> Note that in the Linux kernel tree the OVS_ACTION_ATTR_ADD_MPLS has been
> defined, and to make sure the IDs are in sync, it had to be added to the
> OVS source tree. This required some additional case statements, which
> should be revisited once the OVS implementation is added.
>
> Co-authored-by: Bindiya Kurle <bindiyakurle at gmail.com>
> Co-authored-by: Matteo Croce <mcroce at linux.microsoft.com>
> Signed-off-by: Eelco Chaudron <echaudro at redhat.com>
> Signed-off-by: Bindiya Kurle <bindiyakurle at gmail.com>
> Signed-off-by: Matteo Croce <mcroce at linux.microsoft.com>
> Acked-by: Bindiya Kurle <bindiyakurle at gmail.com>
>
> ---
> v2: - Used definition instead of numeric value in format_dec_ttl_action()
> - Changed format from "dec_ttl(ttl<=1(<actions>)) to
> "dec_ttl(le_1(<actions>))" to be more in line with the check_pkt_len action.
> - Cleaned up format_dec_ttl_action()
> v3:
> - Fixed parsing of "dec_ttl()" action for adding a dp flow.
> - Changed implementation to use the fixed kernel mod implementation
> https://marc.info/?l=linux-netdev&m=160577671609295&w=2
> - Removed introduced force_last flag from odp_execute_actions
> - For now, do not use this new attribute if HW offload is supported, as
> it's causing a performance regression due to HW offload not being
> supported. I will fix this in a separate patch.
> - Added datapath test case for dec_ttl action.
> v4:
> - Fixed some style issues reported by Ilya.
> - Changed OVS_NOT_REACHED() to an ovs_assert(), so information gets logged
> on error.
> - Use batch helper functions rather than access batch->count directly.
> - Fixed memory leak when dec_ttl was the last action in the chain.
> - Fixed do_xlate_actions() return if for exception in backward
> compatibility mode.
> - Added actions parsing test to odp.at
>
> datapath/linux/compat/include/linux/openvswitch.h | 10 ++
> lib/dpif-netdev.c | 2
> lib/dpif.c | 2
> lib/odp-execute.c | 88 +++++++++++++++++++++
> lib/odp-util.c | 45 +++++++++++
> lib/packets.h | 13 +++
> ofproto/ofproto-dpif-ipfix.c | 2
> ofproto/ofproto-dpif-sflow.c | 2
> ofproto/ofproto-dpif-xlate.c | 60 +++++++++++---
> ofproto/ofproto-dpif.c | 40 ++++++++++
> ofproto/ofproto-dpif.h | 6 +
> tests/odp.at | 1
> tests/system-traffic.at | 28 +++++++
> 13 files changed, 282 insertions(+), 17 deletions(-)
>
> diff --git a/datapath/linux/compat/include/linux/openvswitch.h b/datapath/linux/compat/include/linux/openvswitch.h
> index 875de2025..a5357024d 100644
> --- a/datapath/linux/compat/include/linux/openvswitch.h
> +++ b/datapath/linux/compat/include/linux/openvswitch.h
> @@ -1030,6 +1030,8 @@ enum ovs_action_attr {
> OVS_ACTION_ATTR_METER, /* u32 meter number. */
> OVS_ACTION_ATTR_CLONE, /* Nested OVS_CLONE_ATTR_*. */
> OVS_ACTION_ATTR_CHECK_PKT_LEN, /* Nested OVS_CHECK_PKT_LEN_ATTR_*. */
> + OVS_ACTION_ATTR_ADD_MPLS, /* struct ovs_action_add_mpls. */
> + OVS_ACTION_ATTR_DEC_TTL, /* Nested OVS_DEC_TTL_ATTR_*. */
>
> #ifndef __KERNEL__
> OVS_ACTION_ATTR_TUNNEL_PUSH, /* struct ovs_action_push_tnl*/
> @@ -1133,4 +1135,12 @@ struct ovs_zone_limit {
> * keys. False otherwise.
> */
>
> +enum ovs_dec_ttl_attr {
> + OVS_DEC_TTL_ATTR_UNSPEC,
> + OVS_DEC_TTL_ATTR_ACTION, /* Nested struct nlattr */
> + __OVS_DEC_TTL_ATTR_MAX
> +};
> +
> +#define OVS_DEC_TTL_ATTR_MAX (__OVS_DEC_TTL_ATTR_MAX - 1)
> +
> #endif /* _LINUX_OPENVSWITCH_H */
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index 251788b04..c6032e013 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -8046,6 +8046,8 @@ dp_execute_cb(void *aux_, struct dp_packet_batch *packets_,
> case OVS_ACTION_ATTR_CT_CLEAR:
> case OVS_ACTION_ATTR_CHECK_PKT_LEN:
> case OVS_ACTION_ATTR_DROP:
> + case OVS_ACTION_ATTR_DEC_TTL:
> + case OVS_ACTION_ATTR_ADD_MPLS:
> case __OVS_ACTION_ATTR_MAX:
> OVS_NOT_REACHED();
> }
> diff --git a/lib/dpif.c b/lib/dpif.c
> index 26e8bfb7d..a25d8e9ef 100644
> --- a/lib/dpif.c
> +++ b/lib/dpif.c
> @@ -1274,6 +1274,8 @@ dpif_execute_helper_cb(void *aux_, struct dp_packet_batch *packets_,
> case OVS_ACTION_ATTR_UNSPEC:
> case OVS_ACTION_ATTR_CHECK_PKT_LEN:
> case OVS_ACTION_ATTR_DROP:
> + case OVS_ACTION_ATTR_ADD_MPLS:
> + case OVS_ACTION_ATTR_DEC_TTL:
> case __OVS_ACTION_ATTR_MAX:
> OVS_NOT_REACHED();
> }
> diff --git a/lib/odp-execute.c b/lib/odp-execute.c
> index 6eeda2a61..ff2921045 100644
> --- a/lib/odp-execute.c
> +++ b/lib/odp-execute.c
> @@ -719,6 +719,55 @@ odp_execute_sample(void *dp, struct dp_packet *packet, bool steal,
> nl_attr_get_size(subactions), dp_execute_action);
> }
>
> +static bool
> +execute_dec_ttl(struct dp_packet *packet)
> +{
> + struct eth_header *eth = dp_packet_eth(packet);
> +
> + if (dl_type_is_ipv4(eth->eth_type)) {
> + struct ip_header *nh = dp_packet_l3(packet);
> + uint8_t old_ttl = nh->ip_ttl;
> +
> + if (old_ttl <= 1) {
> + return true;
> + }
> +
> + nh->ip_ttl--;
> + nh->ip_csum = recalc_csum16(nh->ip_csum, htons(old_ttl << 8),
> + htons(nh->ip_ttl << 8));
> + } else if (dl_type_is_ipv6(eth->eth_type)) {
> + struct ovs_16aligned_ip6_hdr *nh = dp_packet_l3(packet);
> +
> + if (nh->ip6_hlim <= 1) {
> + return true;
> + }
> +
> + nh->ip6_hlim--;
> + }
> +
> + return false;
> +}
> +
> +static void
> +odp_dec_ttl_exception_handler(void *dp, struct dp_packet_batch *batch,
> + const struct nlattr *action,
> + odp_execute_cb dp_execute_action)
> +{
> + static const struct nl_policy dec_ttl_policy[] = {
> + [OVS_DEC_TTL_ATTR_ACTION] = { .type = NL_A_NESTED },
> + };
> + struct nlattr *attrs[ARRAY_SIZE(dec_ttl_policy)];
> +
> + ovs_assert(nl_parse_nested(action, dec_ttl_policy, attrs,
> + ARRAY_SIZE(attrs))
> + && attrs[OVS_DEC_TTL_ATTR_ACTION]);
> +
> + odp_execute_actions(dp, batch, true,
> + nl_attr_get(attrs[OVS_DEC_TTL_ATTR_ACTION]),
> + nl_attr_get_size(attrs[OVS_DEC_TTL_ATTR_ACTION]),
> + dp_execute_action);
> +}
> +
> static void
> odp_execute_clone(void *dp, struct dp_packet_batch *batch, bool steal,
> const struct nlattr *actions,
> @@ -734,7 +783,7 @@ odp_execute_clone(void *dp, struct dp_packet_batch *batch, bool steal,
> dp_packet_batch_clone(&clone_pkt_batch, batch);
> dp_packet_batch_reset_cutlen(batch);
> odp_execute_actions(dp, &clone_pkt_batch, true, nl_attr_get(actions),
> - nl_attr_get_size(actions), dp_execute_action);
> + nl_attr_get_size(actions), dp_execute_action);
> }
> else {
> odp_execute_actions(dp, batch, true, nl_attr_get(actions),
> @@ -820,6 +869,8 @@ requires_datapath_assistance(const struct nlattr *a)
> case OVS_ACTION_ATTR_CT_CLEAR:
> case OVS_ACTION_ATTR_CHECK_PKT_LEN:
> case OVS_ACTION_ATTR_DROP:
> + case OVS_ACTION_ATTR_ADD_MPLS:
> + case OVS_ACTION_ATTR_DEC_TTL:
> return false;
>
> case OVS_ACTION_ATTR_UNSPEC:
> @@ -979,6 +1030,40 @@ odp_execute_actions(void *dp, struct dp_packet_batch *batch, bool steal,
> }
> break;
>
> + case OVS_ACTION_ATTR_DEC_TTL: {
> + const size_t cnt = dp_packet_batch_size(batch);
> + struct dp_packet_batch invalid_ttl;
> + size_t i;
> +
> + /* Make batch for invalid ttl packets. */
> + dp_packet_batch_init(&invalid_ttl);
> + invalid_ttl.trunc = batch->trunc;
> + invalid_ttl.do_not_steal = batch->do_not_steal;
> +
> + /* Add packets with ttl <=1 to the invalid_ttl batch
> + * and remove it from the batch. */
> + DP_PACKET_BATCH_REFILL_FOR_EACH (i, cnt, packet, batch) {
> + if (execute_dec_ttl(packet)) {
> + dp_packet_batch_add(&invalid_ttl, packet);
> + } else {
> + dp_packet_batch_refill(batch, packet, i);
> + }
> + }
> +
> + /* Execute action on packets with ttl <= 1. */
> + if (!dp_packet_batch_is_empty(&invalid_ttl)) {
> + odp_dec_ttl_exception_handler(dp, &invalid_ttl, a,
> + dp_execute_action);
> + }
> +
> + if (dp_packet_batch_is_empty(batch)) {
> + /* The entire batch was handled as an exception, so we can
> + * stop processing actions. */
> + return;
> + }
> + break;
> + }
> +
> case OVS_ACTION_ATTR_TRUNC: {
> const struct ovs_action_trunc *trunc =
> nl_attr_get_unspec(a, sizeof *trunc);
> @@ -1077,6 +1162,7 @@ odp_execute_actions(void *dp, struct dp_packet_batch *batch, bool steal,
> case OVS_ACTION_ATTR_RECIRC:
> case OVS_ACTION_ATTR_CT:
> case OVS_ACTION_ATTR_UNSPEC:
> + case OVS_ACTION_ATTR_ADD_MPLS:
> case __OVS_ACTION_ATTR_MAX:
> OVS_NOT_REACHED();
> }
> diff --git a/lib/odp-util.c b/lib/odp-util.c
> index e1199d1da..922104e72 100644
> --- a/lib/odp-util.c
> +++ b/lib/odp-util.c
> @@ -143,8 +143,10 @@ odp_action_len(uint16_t type)
> case OVS_ACTION_ATTR_POP_NSH: return 0;
> case OVS_ACTION_ATTR_CHECK_PKT_LEN: return ATTR_LEN_VARIABLE;
> case OVS_ACTION_ATTR_DROP: return sizeof(uint32_t);
> + case OVS_ACTION_ATTR_DEC_TTL: return ATTR_LEN_VARIABLE;
>
> case OVS_ACTION_ATTR_UNSPEC:
> + case OVS_ACTION_ATTR_ADD_MPLS:
> case __OVS_ACTION_ATTR_MAX:
> return ATTR_LEN_INVALID;
> }
> @@ -1110,6 +1112,25 @@ format_odp_check_pkt_len_action(struct ds *ds, const struct nlattr *attr,
> ds_put_cstr(ds, "))");
> }
>
> +static void
> +format_dec_ttl_action(struct ds *ds, const struct nlattr *attr,
> + const struct hmap *portno_names)
> +{
> + const struct nlattr *a;
> + unsigned int left;
> +
> + ds_put_cstr(ds,"dec_ttl(le_1(");
> + NL_ATTR_FOR_EACH (a, left,
> + nl_attr_get(attr), nl_attr_get_size(attr)) {
> + if (nl_attr_type(a) == OVS_DEC_TTL_ATTR_ACTION) {
> + format_odp_actions(ds, nl_attr_get(a),
> + nl_attr_get_size(a), portno_names);
> + break;
> + }
> + }
> + ds_put_format(ds, "))");
> +}
> +
> static void
> format_odp_action(struct ds *ds, const struct nlattr *a,
> const struct hmap *portno_names)
> @@ -1257,7 +1278,11 @@ format_odp_action(struct ds *ds, const struct nlattr *a,
> case OVS_ACTION_ATTR_DROP:
> ds_put_cstr(ds, "drop");
> break;
> + case OVS_ACTION_ATTR_DEC_TTL:
> + format_dec_ttl_action(ds, a, portno_names);
> + break;
> case OVS_ACTION_ATTR_UNSPEC:
> + case OVS_ACTION_ATTR_ADD_MPLS:
> case __OVS_ACTION_ATTR_MAX:
> default:
> format_generic_odp_action(ds, a);
> @@ -2505,6 +2530,26 @@ parse_odp_action__(struct parse_odp_context *context, const char *s,
> return n + 1;
> }
> }
> + {
> + if (!strncmp(s, "dec_ttl(le_1(", 13)) {
> + size_t actions_ofs, ofs;
> + int n = 13;
> +
> + ofs = nl_msg_start_nested(actions, OVS_ACTION_ATTR_DEC_TTL);
> + actions_ofs = nl_msg_start_nested(actions,
> + OVS_DEC_TTL_ATTR_ACTION);
> +
> + int retval = parse_action_list(context, s + n, actions);
> + if (retval < 0) {
> + return retval;
> + }
> +
> + n += retval;
> + nl_msg_end_nested(actions, actions_ofs);
> + nl_msg_end_nested(actions, ofs);
> + return n + 2;
> + }
> + }
>
> {
> if (!strncmp(s, "push_nsh(", 9)) {
> diff --git a/lib/packets.h b/lib/packets.h
> index 481bc22fa..b7fa7b121 100644
> --- a/lib/packets.h
> +++ b/lib/packets.h
> @@ -1211,10 +1211,19 @@ bool in6_is_lla(struct in6_addr *addr);
> void ipv6_multicast_to_ethernet(struct eth_addr *eth,
> const struct in6_addr *ip6);
>
> +static inline bool dl_type_is_ipv4(ovs_be16 dl_type)
> +{
> + return dl_type == htons(ETH_TYPE_IP);
> +}
> +
> +static inline bool dl_type_is_ipv6(ovs_be16 dl_type)
> +{
> + return dl_type == htons(ETH_TYPE_IPV6);
> +}
> +
> static inline bool dl_type_is_ip_any(ovs_be16 dl_type)
> {
> - return dl_type == htons(ETH_TYPE_IP)
> - || dl_type == htons(ETH_TYPE_IPV6);
> + return dl_type_is_ipv4(dl_type) || dl_type_is_ipv6(dl_type);
> }
>
> /* Tunnel header */
> diff --git a/ofproto/ofproto-dpif-ipfix.c b/ofproto/ofproto-dpif-ipfix.c
> index 796eb6f88..1c8e7ae26 100644
> --- a/ofproto/ofproto-dpif-ipfix.c
> +++ b/ofproto/ofproto-dpif-ipfix.c
> @@ -3018,6 +3018,8 @@ dpif_ipfix_read_actions(const struct flow *flow,
> case OVS_ACTION_ATTR_CHECK_PKT_LEN:
> case OVS_ACTION_ATTR_UNSPEC:
> case OVS_ACTION_ATTR_DROP:
> + case OVS_ACTION_ATTR_ADD_MPLS:
> + case OVS_ACTION_ATTR_DEC_TTL:
> case __OVS_ACTION_ATTR_MAX:
> default:
> break;
> diff --git a/ofproto/ofproto-dpif-sflow.c b/ofproto/ofproto-dpif-sflow.c
> index 864c136b5..83989fb90 100644
> --- a/ofproto/ofproto-dpif-sflow.c
> +++ b/ofproto/ofproto-dpif-sflow.c
> @@ -1178,6 +1178,7 @@ dpif_sflow_read_actions(const struct flow *flow,
> case OVS_ACTION_ATTR_CT_CLEAR:
> case OVS_ACTION_ATTR_METER:
> case OVS_ACTION_ATTR_LB_OUTPUT:
> + case OVS_ACTION_ATTR_DEC_TTL:
> break;
>
> case OVS_ACTION_ATTR_SET_MASKED:
> @@ -1226,6 +1227,7 @@ dpif_sflow_read_actions(const struct flow *flow,
> case OVS_ACTION_ATTR_UNSPEC:
> case OVS_ACTION_ATTR_CHECK_PKT_LEN:
> case OVS_ACTION_ATTR_DROP:
> + case OVS_ACTION_ATTR_ADD_MPLS:
> case __OVS_ACTION_ATTR_MAX:
> default:
> break;
> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
> index 7108c8a30..3cac92940 100644
> --- a/ofproto/ofproto-dpif-xlate.c
> +++ b/ofproto/ofproto-dpif-xlate.c
> @@ -40,6 +40,7 @@
> #include "mac-learning.h"
> #include "mcast-snooping.h"
> #include "multipath.h"
> +#include "netdev-offload.h"
> #include "netdev-vport.h"
> #include "netlink.h"
> #include "nx-match.h"
> @@ -4840,9 +4841,12 @@ xlate_controller_action(struct xlate_ctx *ctx, int len,
> enum ofp_packet_in_reason reason,
> uint16_t controller_id,
> uint32_t provider_meter_id,
> - const uint8_t *userdata, size_t userdata_len)
> + const uint8_t *userdata, size_t userdata_len,
> + bool commit)
> {
> - xlate_commit_actions(ctx);
> + if (commit) {
> + xlate_commit_actions(ctx);
> + }
>
> /* A packet sent by an action in a table-miss rule is considered an
> * explicit table miss. OpenFlow before 1.3 doesn't have that concept so
> @@ -5075,10 +5079,6 @@ compose_dec_ttl(struct xlate_ctx *ctx, struct ofpact_cnt_ids *ids)
> {
> struct flow *flow = &ctx->xin->flow;
>
> - if (!is_ip_any(flow)) {
> - return false;
> - }
> -
> ctx->wc->masks.nw_ttl = 0xff;
> if (flow->nw_ttl > 1) {
> flow->nw_ttl--;
> @@ -5088,7 +5088,8 @@ compose_dec_ttl(struct xlate_ctx *ctx, struct ofpact_cnt_ids *ids)
>
> for (i = 0; i < ids->n_controllers; i++) {
> xlate_controller_action(ctx, UINT16_MAX, OFPR_INVALID_TTL,
> - ids->cnt_ids[i], UINT32_MAX, NULL, 0);
> + ids->cnt_ids[i], UINT32_MAX, NULL, 0,
> + true);
> }
>
> /* Stop processing for current table. */
> @@ -5129,7 +5130,7 @@ compose_dec_nsh_ttl_action(struct xlate_ctx *ctx)
> return false;
> } else {
> xlate_controller_action(ctx, UINT16_MAX, OFPR_INVALID_TTL,
> - 0, UINT32_MAX, NULL, 0);
> + 0, UINT32_MAX, NULL, 0, true);
> }
> }
>
> @@ -5162,7 +5163,7 @@ compose_dec_mpls_ttl_action(struct xlate_ctx *ctx)
> return false;
> } else {
> xlate_controller_action(ctx, UINT16_MAX, OFPR_INVALID_TTL, 0,
> - UINT32_MAX, NULL, 0);
> + UINT32_MAX, NULL, 0, true);
> }
> }
>
> @@ -5186,6 +5187,40 @@ xlate_delete_field(struct xlate_ctx *ctx,
> ds_destroy(&s);
> }
>
> +/* New handling for dec_ttl action. */
'New handling' makes sense in a patch, but doesn't while reading the code.
> +static bool
> +xlate_dec_ttl_action(struct xlate_ctx *ctx, struct ofpact_cnt_ids *ids)
> +{
> + struct flow *flow = &ctx->xin->flow;
> + size_t offset, offset_actions;
> + size_t i;
> +
> + if (!is_ip_any(flow)) {
> + return false;
> + }
> +
> + if (!ctx->xbridge->support.dec_ttl_action
> + || netdev_is_flow_api_enabled()) {
> + return compose_dec_ttl(ctx, ids);
> + }
> +
> + xlate_commit_actions(ctx);
> + offset = nl_msg_start_nested(ctx->odp_actions, OVS_ACTION_ATTR_DEC_TTL);
> + offset_actions = nl_msg_start_nested(ctx->odp_actions,
> + OVS_DEC_TTL_ATTR_ACTION);
> +
> + for (i = 0; i < ids->n_controllers; i++) {
> + xlate_controller_action(ctx, UINT16_MAX, OFPR_INVALID_TTL,
> + ids->cnt_ids[i], UINT32_MAX, NULL, 0, false);
> + }
> +
> + nl_msg_end_nested(ctx->odp_actions, offset_actions);
> + nl_msg_end_nested(ctx->odp_actions, offset);
> +
> + xlate_commit_actions(ctx);
> + return false;
> +}
> +
> /* Emits an action that outputs to 'port', within 'ctx'.
> *
> * 'controller_len' affects only packets sent to an OpenFlow controller. It
> @@ -5236,7 +5271,7 @@ xlate_output_action(struct xlate_ctx *ctx, ofp_port_t port,
> : group_bucket_action ? OFPR_GROUP
> : ctx->in_action_set ? OFPR_ACTION_SET
> : OFPR_ACTION),
> - 0, UINT32_MAX, NULL, 0);
> + 0, UINT32_MAX, NULL, 0, true);
> break;
> case OFPP_NONE:
> break;
> @@ -6797,7 +6832,7 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len,
> controller->controller_id,
> controller->provider_meter_id,
> controller->userdata,
> - controller->userdata_len);
> + controller->userdata_len, true);
> }
> break;
>
> @@ -7005,8 +7040,7 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len,
> break;
>
> case OFPACT_DEC_TTL:
> - wc->masks.nw_ttl = 0xff;
> - if (compose_dec_ttl(ctx, ofpact_get_DEC_TTL(a))) {
> + if (xlate_dec_ttl_action(ctx, ofpact_get_DEC_TTL(a))) {
> return;
> }
> break;
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index fd0b2fdea..49e026359 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -1186,6 +1186,45 @@ check_trunc_action(struct dpif_backer *backer)
> return !error;
> }
>
> +/* Tests whether 'dpif' datapath supports decrement of the IP TTL via
> + * OVS_ACTION_DEC_TTL. */
> +static bool
> +check_dec_ttl_action(struct dpif *dpif)
> +{
> + struct odputil_keybuf keybuf;
> + struct flow flow = { 0 };
It's probbaly better to just memset it as in other similar functions
to avoid compiler's complains.
> + struct ofpbuf actions;
> + uint32_t actbuf[4];
> + struct ofpbuf key;
> + bool supported;
> + size_t start;
> +
> + struct odp_flow_key_parms odp_parms = {
> + .flow = &flow,
> + .probe = true,
> + };
> +
> + ofpbuf_use_stack(&key, &keybuf, sizeof keybuf);
> + odp_flow_key_from_flow(&odp_parms, &key);
> +
> + ofpbuf_use_stack(&actions, &actbuf, sizeof actbuf);
> + start = nl_msg_start_nested(&actions, OVS_ACTION_ATTR_DEC_TTL);
> + nl_msg_put_flag(&actions, OVS_DEC_TTL_ATTR_ACTION);
> + nl_msg_end_nested(&actions, start);
> +
> + supported = dpif_probe_feature(dpif, "dec_ttl", &key, &actions, NULL);
> +
> + if (supported) {
> + VLOG_INFO("%s: Datapath supports dec_ttl action",
> + dpif_name(dpif));
> + } else {
> + VLOG_INFO("%s: Datapath does not support dec_ttl action",
> + dpif_name(dpif));
> + }
> +
> + return supported;
> +}
> +
> /* Tests whether 'backer''s datapath supports the clone action
> * OVS_ACTION_ATTR_CLONE. */
> static bool
> @@ -1590,6 +1629,7 @@ check_support(struct dpif_backer *backer)
> dpif_supports_explicit_drop_action(backer->dpif);
> backer->rt_support.lb_output_action=
> dpif_supports_lb_output_action(backer->dpif);
> + backer->rt_support.dec_ttl_action = check_dec_ttl_action(backer->dpif);
During discussions about all-zero SNAT feature support I remembered that
we have a 'capabilities' table that should reflect all the datapath
supported fetures. So, this should be added there as well. And documented
in vswitchd/vswitch.xml.
>
> /* 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 b41c3d82a..f4d1152bc 100644
> --- a/ofproto/ofproto-dpif.h
> +++ b/ofproto/ofproto-dpif.h
> @@ -204,7 +204,11 @@ struct group_dpif *group_dpif_lookup(struct ofproto_dpif *,
> DPIF_SUPPORT_FIELD(bool, explicit_drop_action, "Explicit Drop action") \
> \
> /* True if the datapath supports balance_tcp optimization */ \
> - DPIF_SUPPORT_FIELD(bool, lb_output_action, "Optimized Balance TCP mode")
> + DPIF_SUPPORT_FIELD(bool, lb_output_action, "Optimized Balance TCP mode")\
> + \
> + /* True if the datapath supports dec_ttl action. */ \
> + DPIF_SUPPORT_FIELD(bool, dec_ttl_action, "Decrement TTL action")
> +
>
>
> /* Stores the various features which the corresponding backer supports. */
> diff --git a/tests/odp.at b/tests/odp.at
> index b762ebb2b..24946bec4 100644
> --- a/tests/odp.at
> +++ b/tests/odp.at
> @@ -384,6 +384,7 @@ check_pkt_len(size=200,gt(drop),le(5))
> check_pkt_len(size=200,gt(ct(nat)),le(drop))
> check_pkt_len(size=200,gt(set(eth(src=00:01:02:03:04:05,dst=10:11:12:13:14:15))),le(set(eth(src=00:01:02:03:04:06,dst=10:11:12:13:14:16))))
> lb_output(1)
> +dec_ttl(le_1(userspace(pid=3614533484,controller(reason=2,dont_send=0,continuation=0,recirc_id=1,rule_cookie=0,controller_id=0,max_len=65535))))
Maybe it will make sense to also add a very simple variant of this action,
e.g. dec_ttl(le_1(drop)).
> ])
> AT_CHECK_UNQUOTED([ovstest test-odp parse-actions < actions.txt], [0],
> [`cat actions.txt`
> diff --git a/tests/system-traffic.at b/tests/system-traffic.at
> index fb5b9a36d..ef9b555b6 100644
> --- a/tests/system-traffic.at
> +++ b/tests/system-traffic.at
> @@ -1400,6 +1400,34 @@ AT_CHECK([ovs-ofctl dump-flows br0 | grep "in_port=4" | ofctl_strip], [0], [dnl
> OVS_TRAFFIC_VSWITCHD_STOP
> AT_CLEANUP
>
> +AT_SETUP([datapath - dec_ttl action])
> +OVS_TRAFFIC_VSWITCHD_START()
> +
> +AT_CHECK([ovs-ofctl add-flow br0 "actions=normal"])
> +
> +ADD_NAMESPACES(at_ns0, at_ns1)
> +
> +ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24")
> +ADD_VETH(p1, at_ns1, br0, "10.1.1.2/24")
> +
> +AT_CHECK([ovs-vsctl -- set interface ovs-p0 ofport_request=1 \
> + -- set interface ovs-p1 ofport_request=2])
> +
> +AT_CHECK([ovs-ofctl del-flows br0])
> +AT_CHECK([ovs-ofctl add-flow br0 in_port=1,action=dec_ttl,2])
> +AT_CHECK([ovs-ofctl add-flow br0 in_port=2,action=dec_ttl,1])
> +
> +NS_CHECK_EXEC([at_ns0], [ping -c 1 -w 2 10.1.1.2 | grep -o -E 'ttl=[[[:digit:]]]+'], [0], [dnl
> +ttl=63
> +])
> +
> +NS_CHECK_EXEC([at_ns0], [ping -q -c 1 -t 1 10.1.1.2 | FORMAT_PING], [0], [dnl
> +1 packets transmitted, 0 received, 100% packet loss, time 0ms
> +])
> +
> +OVS_TRAFFIC_VSWITCHD_STOP
> +AT_CLEANUP
> +
> AT_BANNER([conntrack])
>
> AT_SETUP([conntrack - controller])
>
More information about the dev
mailing list