[ovs-dev] [PATCH 3/3] Handle tunnel set changes for BUM traffic in the vtep emulator

[Guru Shetty]

On 12 February 2016 at 17:12, Darrell Ball <dlu998 at gmail.com> wrote:

> Signed-off-by: Darrell Ball <dball at vmware.com>
>

Other than Russell's comments:

Please add a descriptive comment on the problem and your solution. I was
looking at a p0 bug with ovs-vtep recently and I couldn't understand why
something was changed (by myself). A descriptive commit message helps a lot.

pep8 on the file shows the following new issues with formatting ( There are
some older issues that I think needs to be fixed separately).


vtep/ovs-vtep:147:19: E128 continuation line under-indented for visual
indent
vtep/ovs-vtep:150:16: E111 indentation is not a multiple of four
vtep/ovs-vtep:151:16: E111 indentation is not a multiple of four




> ---
>  vtep/ovs-vtep | 20 ++++++++++++++++++--
>  1 file changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/vtep/ovs-vtep b/vtep/ovs-vtep
> index 46a5692..54bf0db 100755
> --- a/vtep/ovs-vtep
> +++ b/vtep/ovs-vtep
> @@ -135,15 +135,31 @@ class Logical_Switch(object):
>              ovs_ofctl("add-flow %s
> table=1,priority=1,in_port=%s,action=%s"
>                          % (self.short_name, port_no,
> ",".join(flood_ports)))
>
> +        existing_flow = 0
> +        appended_flood_tunnel_port = 0
> +        flows = ovs_ofctl("dump-flows %s table=1,cookie=0x4848/-1"
> +                  % self.short_name).splitlines()
> +        for f in flows:
> +            if ("table" in f):
> +               existing_flow = 1
> +               break
> +
>          # Traffic coming from a VTEP physical port should only be flooded
> to
>          # one 'unknown-dst' and to all other physical ports that belong
> to that
>          # VTEP device and this logical switch.
>          for tunnel in self.unknown_dsts:
>              port_no = self.tunnels[tunnel][0]
>              flood_ports.append(port_no)
> +            appended_flood_tunnel_port = 1
>              break
> -
> -        ovs_ofctl("add-flow %s table=1,priority=0,action=%s"
> +        # Handle remote tunnel port set changes
> +        if ((existing_flow == 1) and (appended_flood_tunnel_port == 1)):
> +            ovs_ofctl(
> +                  "mod-flows %s
> table=1,priority=0,cookie=0x4848/-1,action=%s"
> +                  % (self.short_name, ",".join(flood_ports)))
> +        elif (existing_flow == 0):
> +            ovs_ofctl(
> +                  "add-flow %s table=1,priority=0,cookie=0x4848,action=%s"
>                    % (self.short_name, ",".join(flood_ports)))
>

I am not quite sure I understand the problem here. 'add-flow' with same
match but different actions should update the flow automatically. What is
the case when you need this? You can see that update_flood is called from
numerous places. Why is this a problem only with " tunnel set changes"?


>
>      def add_lbinding(self, lbinding):
> --
> 1.9.1
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>