I would urge Ben to check this up as well, though. Jarno
> On Nov 11, 2015, at 2:23 PM, Jarno Rajahalme <ja...@ovn.org> wrote: > > With a comment below, > > Acked-by: Jarno Rajahalme <ja...@ovn.org> > >> On Nov 11, 2015, at 11:39 AM, Joe Stringer <joestrin...@nicira.com> wrote: >> >> Add an ofproto-level function to allow implementations to reject >> specific action types based on internal implementation details. The >> first user will be the next patch, which checks for datapath (kernel) >> support for various aspects of connection tracking and uses this to >> allow or reject ct() actions. >> >> Signed-off-by: Joe Stringer <joestrin...@nicira.com> >> CC: Jarno Rajahalme <ja...@ovn.org> >> --- >> ofproto/ofproto-dpif.c | 1 + >> ofproto/ofproto-provider.h | 14 ++++++++++++++ >> ofproto/ofproto.c | 9 +++++++++ >> 3 files changed, 24 insertions(+) >> >> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c >> index f0a2ca59e2e8..8b1760c95409 100644 >> --- a/ofproto/ofproto-dpif.c >> +++ b/ofproto/ofproto-dpif.c >> @@ -5689,6 +5689,7 @@ const struct ofproto_class ofproto_dpif_class = { >> port_poll_wait, >> port_is_lacp_current, >> port_get_lacp_stats, >> + NULL, >> NULL, /* rule_choose_table */ >> rule_alloc, >> rule_construct, >> diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h >> index 117cd1fcc02e..da6cb37c0b60 100644 >> --- a/ofproto/ofproto-provider.h >> +++ b/ofproto/ofproto-provider.h >> @@ -1077,6 +1077,20 @@ struct ofproto_class { >> /* ## OpenFlow Rule Functions ## */ >> /* ## ----------------------- ## */ >> >> + /* Checks whether the action 'ofpact' is supported by 'ofproto'. On >> + * success, returns 0. On failure, returns an OpenFlow error code. >> + * >> + * Some actions are marked as optional by the OpenFlow specification. >> + * Furthermore, OVS includes support for several vendor extensions which >> + * may not be supported by all ofproto implementations. This function >> + * allows specific actions to be rejected based on internal datapath >> + * support. Failure implies that an OpenFlow rule that includes 'ofpact' >> + * in its actions can never be inserted into 'ofproto'. >> + * >> + * If this function is NULL then all actions are supported. */ >> + enum ofperr (*rule_check_action)(const struct ofproto *ofproto, >> + const struct ofpact *ofpact); >> + >> /* Chooses an appropriate table for 'match' within 'ofproto'. On >> * success, stores the table ID into '*table_idp' and returns 0. On >> * failure, returns an OpenFlow error code. >> diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c >> index c7dd8a2c991b..338330108df1 100644 >> --- a/ofproto/ofproto.c >> +++ b/ofproto/ofproto.c >> @@ -3346,10 +3346,19 @@ ofproto_check_ofpacts(struct ofproto *ofproto, >> } >> >> OFPACT_FOR_EACH (a, ofpacts, ofpacts_len) { >> + enum ofperr error; >> + >> if (a->type == OFPACT_GROUP >> && !ofproto_group_exists(ofproto, ofpact_get_GROUP(a)->group_id)) >> { >> return OFPERR_OFPBAC_BAD_OUT_GROUP; >> } >> + >> + if (ofproto->ofproto_class->rule_check_action) { >> + error = ofproto->ofproto_class->rule_check_action(ofproto, a); >> + if (error) { >> + return error; >> + } >> + } > > I would make this call for ‘known to possibly not be supported’ actions, in > the spirit of the group check above. > >> } >> >> return 0; >> -- >> 2.1.4 >> > _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev