On 30 Jun 2023, at 21:05, Eric Garver wrote:
> This is prep for adding a different OVS_ACTION_ATTR_ enum value. This
> action, OVS_ACTION_ATTR_DEC_TTL, is not actually implemented. However,
> to make -Werror happy we must add a case to all existing switches.
>
> Signed-off-by: Eric Garver <e...@garver.life>
> ---
> include/linux/openvswitch.h | 1 +
> lib/dpif-netdev.c | 1 +
> lib/dpif.c | 1 +
> lib/odp-execute.c | 2 ++
> lib/odp-util.c | 2 ++
> ofproto/ofproto-dpif-ipfix.c | 1 +
> ofproto/ofproto-dpif-sflow.c | 1 +
> 7 files changed, 9 insertions(+)
>
> diff --git a/include/linux/openvswitch.h b/include/linux/openvswitch.h
> index e305c331516b..a265e05ad253 100644
> --- a/include/linux/openvswitch.h
> +++ b/include/linux/openvswitch.h
> @@ -1085,6 +1085,7 @@ enum ovs_action_attr {
> 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*/
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index abe63412ebfc..565cde5f5010 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -9193,6 +9193,7 @@ dp_execute_cb(void *aux_, struct dp_packet_batch
> *packets_,
> case OVS_ACTION_ATTR_POP_NSH:
> case OVS_ACTION_ATTR_CT_CLEAR:
> case OVS_ACTION_ATTR_CHECK_PKT_LEN:
> + case OVS_ACTION_ATTR_DEC_TTL:
> case OVS_ACTION_ATTR_DROP:
> case OVS_ACTION_ATTR_ADD_MPLS:
nit: I would add it here, to keep it in the same order ad the enum definition.
> case __OVS_ACTION_ATTR_MAX:
> diff --git a/lib/dpif.c b/lib/dpif.c
> index b1cbf39c48d6..d328bf288de0 100644
> --- a/lib/dpif.c
> +++ b/lib/dpif.c
> @@ -1281,6 +1281,7 @@ dpif_execute_helper_cb(void *aux_, struct
> dp_packet_batch *packets_,
> case OVS_ACTION_ATTR_CT_CLEAR:
> case OVS_ACTION_ATTR_UNSPEC:
> case OVS_ACTION_ATTR_CHECK_PKT_LEN:
> + case OVS_ACTION_ATTR_DEC_TTL:
> case OVS_ACTION_ATTR_DROP:
> case OVS_ACTION_ATTR_ADD_MPLS:
nit: I would add it here, to keep it in the same order ad the enum definition.
> case __OVS_ACTION_ATTR_MAX:
> diff --git a/lib/odp-execute.c b/lib/odp-execute.c
> index 37f0f717af61..86f1ccb39bf9 100644
> --- a/lib/odp-execute.c
> +++ b/lib/odp-execute.c
> @@ -837,6 +837,7 @@ requires_datapath_assistance(const struct nlattr *a)
I would put the below DEC_TTL here, although it’s not used due to design
issues, but if it ever would be, it should be here.
> return false;
>
> case OVS_ACTION_ATTR_UNSPEC:
> + case OVS_ACTION_ATTR_DEC_TTL:
> case __OVS_ACTION_ATTR_MAX:
> OVS_NOT_REACHED();
> }
> @@ -1223,6 +1224,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_DEC_TTL:
> case __OVS_ACTION_ATTR_MAX:
> /* The following actions are handled by the scalar implementation. */
> case OVS_ACTION_ATTR_POP_VLAN:
> diff --git a/lib/odp-util.c b/lib/odp-util.c
> index 2ec889c417e5..e1ca2cd0e02b 100644
> --- a/lib/odp-util.c
> +++ b/lib/odp-util.c
> @@ -146,6 +146,7 @@ odp_action_len(uint16_t type)
> case OVS_ACTION_ATTR_DROP: return sizeof(uint32_t);
>
> case OVS_ACTION_ATTR_UNSPEC:
> + case OVS_ACTION_ATTR_DEC_TTL:
This can be ‘case OVS_ACTION_ATTR_DEC_TTL: return ATTR_LEN_VARIABLE;’
> case __OVS_ACTION_ATTR_MAX:
> return ATTR_LEN_INVALID;
> }
> @@ -1287,6 +1288,7 @@ format_odp_action(struct ds *ds, const struct nlattr *a,
> ds_put_cstr(ds, "drop");
> break;
> case OVS_ACTION_ATTR_UNSPEC:
> + case OVS_ACTION_ATTR_DEC_TTL:
I think we should be able to add the format for dec_ttl just in case it gets
added through some other means.
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, "))");
}
case OVS_ACTION_ATTR_DEC_TTL:
format_dec_ttl_action(ds, a, portno_names);
break;
> case __OVS_ACTION_ATTR_MAX:
> default:
> format_generic_odp_action(ds, a);
> diff --git a/ofproto/ofproto-dpif-ipfix.c b/ofproto/ofproto-dpif-ipfix.c
> index e6c2968f7e90..c71880e18122 100644
> --- a/ofproto/ofproto-dpif-ipfix.c
> +++ b/ofproto/ofproto-dpif-ipfix.c
> @@ -3133,6 +3133,7 @@ dpif_ipfix_read_actions(const struct flow *flow,
> case OVS_ACTION_ATTR_POP_NSH:
> case OVS_ACTION_ATTR_CHECK_PKT_LEN:
> case OVS_ACTION_ATTR_UNSPEC:
> + case OVS_ACTION_ATTR_DEC_TTL:
> case OVS_ACTION_ATTR_DROP:
> case OVS_ACTION_ATTR_ADD_MPLS:
nit: I would add it here, to keep it in the same order ad the enum definition.
> case __OVS_ACTION_ATTR_MAX:
> diff --git a/ofproto/ofproto-dpif-sflow.c b/ofproto/ofproto-dpif-sflow.c
> index a405eb0563fe..f026cf0c815c 100644
> --- a/ofproto/ofproto-dpif-sflow.c
> +++ b/ofproto/ofproto-dpif-sflow.c
> @@ -1228,6 +1228,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_DEC_TTL:
> case OVS_ACTION_ATTR_ADD_MPLS:
nit: I would add it here, to keep it in the same order ad the enum definition.
> case __OVS_ACTION_ATTR_MAX:
> default:
> --
> 2.39.0
>
> _______________________________________________
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
