On Thu, Jul 06, 2023 at 10:40:18AM +0200, Eelco Chaudron wrote: > > > 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.
It can either be before _DROP or after _ADD_MPLS. In either case it's out of order. :P [..] > > 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. ACK. [..] > > 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;’ ACK. > > 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, "))"); > } Sure. I guess this will be compile only tested. [..] _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev