[ovs-dev] [PATCH ovn 1/4] ovn-controller: A new action "select".

[Han Zhou]

On Fri, Jan 17, 2020 at 4:38 AM Numan Siddique <numans at ovn.org> wrote:
>
> On Tue, Jan 7, 2020 at 8:00 AM Han Zhou <hzhou at ovn.org> wrote:
> >
> > Support a new logical flow action "select", which can be used to
> > implement features such as ECMP.  The action uses OpenFlow group
> > action to select an integer (uint16_t) from a list of integers,
> > and assign it to specified field, e.g.:
> >     select(reg0, 1, 2, 3)
> > A weight can be specified for each member as well, e.g.:
> >     select(reg0, 1=20, 2=30, 3=50)
> >
> > Signed-off-by: Han Zhou <hzhou at ovn.org>
>
>
> Hi Han,
>
> The overall series LGTM. I have one comment on the action "select".
> We already have existing actions  like put_dhcp_opts (and others)
> which store the result to a register.
> I think it would be better to use it in similar way i.e "reg0 =
> select(1:20, 2:30, 3:50)".
>
Thanks Numan!

In fact I was trying to do exactly the same as what you are suggesting.
However, the OF group action is special that the actions executed in group
buckets can't save any changes made to registers, metadata or stack out of
the action execution. See documentation details in this commit:
https://github.com/openvswitch/ovs/commit/afc0c379f6e13cdcbb4447f17bbb74b882503af4

Because of this, we can't exit the action, and resubmit is used as the last
action in each bucket to recursively execute the next pipelines with the
new values of the registers. For the same reason, we have to change the
register inside the bucket, instead of using "=" outside of the action.

> Also instead of "=" I would suggest to use ":" to store the weight.

I was also using ":" in the beginning, but it causes problem in lexer
parsing, because the lex_parse_integer() treats ":" as part of a token so
that it can parse a MAC address or a <IP>:<port> token (although these are
not integers).
/* Find the extent of an "integer" token, which can be in decimal or
 * hexadecimal, or an Ethernet address or IPv4 or IPv6 address, as 'start'
 * through 'end'.
...

So this is why I changed to use "=" instead.
Does this make sense to you?

Thanks,
Han