On Fri, Oct 12, 2012 at 04:31:52PM +0900, Isaku Yamahata wrote:
> On Fri, Oct 12, 2012 at 09:26:28AM +0900, Simon Horman wrote:
> > Internally a new_cookie value UINT64_MAX is used for
> > an OpenFlow 1.2 flow mod whose command is not Add.
> > Open Flow 1.2 does not permit adding flows for such commands.
> > Also, UINT64_MAX is a prohibited cookie value, and the
> > existing code created a flow with that value as for the cookie.
>
> > Signed-off-by: Simon Horman <ho...@verge.net.au>
> > ---
> > ofproto/ofproto.c | 15 +++++++++++++--
> > 1 file changed, 13 insertions(+), 2 deletions(-)
> >
> > diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
> > index 97e00ab..c712bf9 100644
> > --- a/ofproto/ofproto.c
> > +++ b/ofproto/ofproto.c
> > @@ -3081,6 +3081,17 @@ modify_flows__(struct ofproto *ofproto, struct
> > ofconn *ofconn,
> > return error;
> > }
> >
> > +static enum ofperr
> > +modify_flows_add(struct ofproto *ofproto, struct ofconn *ofconn,
> > + const struct ofputil_flow_mod *fm,
> > + const struct ofp_header *request)
> > +{
> > + if (fm->cookie_mask != htonll(0) || fm->new_cookie ==
> > htonll(UINT64_MAX)) {
> > + return 0;
> > + }
>
> This depends on how to interpret the spec...
>
> The OF1.2 says A.3.4.1
> > The value -1 (0xffffffffffffffff) is reserved and must not be used.
>
> return an error when UINT64_MAX?
> But I can't find any suitable error code, but OFPFMFC_UNKNOWN.
>
> Do we need to check if cookie mask is non zero?
> The spec says
> > This field is ignored by OFPC_ADD messages.
Simon's change only affects OpenFlow 1.2 OFPFC_MODIFY and
OFPFC_MODIFY_STRICT requests that do not match any existing flow in
the flow table. For those requests, fm->new_cookie will always be
UINT64_MAX (it doesn't come from the OpenFlow message in this case).
The behavior that Simon implements is correct for OpenFlow 1.2, which
says that a modify request that matches no flows has no effect and is
not an error.
Simon's patch doesn't have any effect on OFPFC_ADD, because
modify_flows_add() won't be called for OFPFC_ADD.
I applied this to master.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
