On Wed, Jul 29, 2015 at 08:40:04AM -0700, Ben Pfaff wrote: > On Tue, Jul 28, 2015 at 10:32:50PM -0300, Flavio Leitner wrote: > > On Tue, Jul 28, 2015 at 03:22:46PM -0700, Ben Pfaff wrote: > > > This is implied by the list of ports that are valid for output in the > > > various versions of the OpenFlow specification. > > > > > > Found by OFTest. > > > > Looks good to me. > > I wonder how one could set the output port to none. > > > > Acked-by: Flavio Leitner <[email protected]> > > Thanks for the review. I took a closer look on this one too and ended > up adding a pair of tests and adjusting the "bundle" action, which used > ofpact_check_output_port() but had specific behavior for OFPP_NONE. > Here's what I applied to master and branch-2.4:
Right, OFPP_NONE means no ports are acceptable there. good catch. fbl > > --8<--------------------------cut here-------------------------->8-- > > From: Ben Pfaff <[email protected]> > Date: Wed, 29 Jul 2015 08:36:07 -0700 > Subject: [PATCH] ofp-actions: OFPP_ANY (aka OFPP_NONE) is not a valid output > port. > > This is implied by the list of ports that are valid for output in the > various versions of the OpenFlow specification. > > Found by OFTest. > > Signed-off-by: Ben Pfaff <[email protected]> > Acked-by: Flavio Leitner <[email protected]> > --- > lib/bundle.c | 14 +++++++------- > lib/ofp-actions.c | 4 +++- > tests/ofp-actions.at | 8 ++++++++ > 3 files changed, 18 insertions(+), 8 deletions(-) > > diff --git a/lib/bundle.c b/lib/bundle.c > index f1b1478..baf6bbf 100644 > --- a/lib/bundle.c > +++ b/lib/bundle.c > @@ -1,4 +1,4 @@ > -/* Copyright (c) 2011, 2012, 2013, 2014 Nicira, Inc. > +/* Copyright (c) 2011, 2012, 2013, 2014, 2015 Nicira, Inc. > * > * Licensed under the Apache License, Version 2.0 (the "License"); > * you may not use this file except in compliance with the License. > @@ -117,14 +117,14 @@ bundle_check(const struct ofpact_bundle *bundle, > ofp_port_t max_ports, > > for (i = 0; i < bundle->n_slaves; i++) { > ofp_port_t ofp_port = bundle->slaves[i]; > - enum ofperr error; > > - error = ofpact_check_output_port(ofp_port, max_ports); > - if (error) { > - VLOG_WARN_RL(&rl, "invalid slave %"PRIu16, ofp_port); > - return error; > + if (ofp_port != OFPP_NONE) { > + enum ofperr error = ofpact_check_output_port(ofp_port, > max_ports); > + if (error) { > + VLOG_WARN_RL(&rl, "invalid slave %"PRIu16, ofp_port); > + return error; > + } > } > - > /* Controller slaves are unsupported due to the lack of a max_len > * argument. This may or may not change in the future. There doesn't > * seem to be a real-world use-case for supporting it. */ > diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c > index eef3389..14a2802 100644 > --- a/lib/ofp-actions.c > +++ b/lib/ofp-actions.c > @@ -5396,10 +5396,12 @@ ofpact_check_output_port(ofp_port_t port, ofp_port_t > max_ports) > case OFPP_FLOOD: > case OFPP_ALL: > case OFPP_CONTROLLER: > - case OFPP_NONE: > case OFPP_LOCAL: > return 0; > > + case OFPP_NONE: > + return OFPERR_OFPBAC_BAD_OUT_PORT; > + > default: > if (ofp_to_u16(port) < ofp_to_u16(max_ports)) { > return 0; > diff --git a/tests/ofp-actions.at b/tests/ofp-actions.at > index 39401f3..6308682 100644 > --- a/tests/ofp-actions.at > +++ b/tests/ofp-actions.at > @@ -135,6 +135,9 @@ ffff 0018 00002320 001d 3039 00005BA0 00008707 0000B26E > & ofp_actions|WARN|OpenFlow action NXAST_DEC_TTL_CNT_IDS length 17 is not a > multiple of 8 > ffff 0011 00002320 0015 0001 00000000 0000000000000000 > > +# bad OpenFlow10 actions: OFPBAC_BAD_OUT_PORT > +0000 0008 ffff 0000 > + > ]) > sed '/^[[#&]]/d' < test-data > input.txt > sed -n 's/^# //p; /^$/p' < test-data > expout > @@ -297,6 +300,11 @@ ffff 0020 00002320 0015 000500000000 80003039005A02fd > 0400000000000000 > # > actions=sample(probability=12345,collector_set_id=23456,obs_domain_id=34567,obs_point_id=45678) > ffff 0018 00002320 001d 3039 00005BA0 00008707 0000B26E > > +# bad OpenFlow11 actions: OFPBAC_BAD_OUT_PORT > +& ofp_actions|WARN|bad action at offset 0 (OFPBAC_BAD_OUT_PORT): > +& 00000000 00 00 00 10 ff ff ff ff-00 00 00 00 00 00 00 00 > +0000 0010 ffffffff 0000 000000000000 > + > ]) > sed '/^[[#&]]/d' < test-data > input.txt > sed -n 's/^# //p; /^$/p' < test-data > expout > -- > 2.1.3 > > _______________________________________________ > dev mailing list > [email protected] > http://openvswitch.org/mailman/listinfo/dev _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
