On 3/2/16, 11:22 AM, "dev on behalf of Guru Shetty"
<dev-boun...@openvswitch.org on behalf of g...@ovn.org> wrote:
>On 12 February 2016 at 17:12, Darrell Ball <dlu...@gmail.com> wrote:
>
>> Signed-off-by: Darrell Ball <db...@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?
Thanks for looking
>
This was intended to handle tunnel set changes in ovs-vtep, such as one tunnel
coming into the set.
When I wrote this code in Dec/2015 I noticed add-flow did not update the flood
set
when it changed, so I added the mod-flow for modify cases and it always worked.
I just re-tested this now, albeit with a different environment and verified
that add-flow is able to update the
flood set from ovs-vtep, so it would seem special mod-flow checking and
handling is unnecessary.
I am not sure what is different now; I even tried with and without cookies and
the result is the same
darrrell@darrrell-Standard-PC-i440FX-PIIX-1996:~/openvswitch/ovs$ cat
/usr/local/var/log/openvswitch/ovs-vtep.log
.
.
.
2016-03-02T06:11:25.320Z | 6 | ovs-vtep | INFO | DARRELL8888: time taken 2:
2144
2016-03-02T06:11:25.320Z | 7 | ovs-vtep | INFO | DARRELL: add-flow for LS
vtep_ls1 to 1 <<<<<<<<<<<<<<<<<
2016-03-02T06:11:25.334Z | 8 | ovs-vtep | INFO | adding tunnel
vxlan_over_ipv4/10.10.1.11
.
.
2016-03-02T06:11:25.396Z | 11 | ovs-vtep | INFO | DARRELL8888: time taken 2:
2227
2016-03-02T06:11:25.396Z | 12 | ovs-vtep | INFO | DARRELL: add-flow for LS
vtep_ls1 to 1,2 <<<<<<<<<<<<<<<
2016-03-02T06:11:25.436Z | 13 | ovs-vtep | INFO | creating tunnel record in
vtep for remote_ip:”10.10.1.11"
darrrell@darrrell-Standard-PC-i440FX-PIIX-1996:~/openvswitch/ovs$ sudo
ovs-ofctl dump-flows vtep_ls1
NXST_FLOW reply (xid=0x4):
cookie=0x0, duration=24.572s, table=0, n_packets=0, n_bytes=0, idle_age=24,
in_port=1
actions=learn(table=1,idle_timeout=15,priority=1000,cookie=0x5000,NXM_OF_ETH_DST[]=NXM_OF_ETH_SRC[],output:NXM_OF_IN_PORT[]),resubmit(,1)
cookie=0x0, duration=24.501s, table=0, n_packets=0, n_bytes=0, idle_age=24,
priority=1000,in_port=2 actions=resubmit(,1)
cookie=0x0, duration=24.604s, table=0, n_packets=0, n_bytes=0, idle_age=24,
priority=0 actions=drop
cookie=0x0, duration=24.499s, table=1, n_packets=0, n_bytes=0, idle_age=24,
priority=1000,dl_dst=52:54:00:aa:90:55 actions=output:2
cookie=0x0, duration=24.496s, table=1, n_packets=0, n_bytes=0, idle_age=24,
priority=1,in_port=2 actions=output:1
cookie=0x0, duration=24.490s, table=1, n_packets=0, n_bytes=0, idle_age=24,
priority=0 actions=output:1,output:2 <<<<<<<<<<<<<<<
>You can see that update_flood is called from
>numerous places. Why is this a problem only with " tunnel set changes"?
I intended to limit the patch for remote handling
However, I did mention local set changes as something to look into in the Jan
11 cover letter.
However, given that add-flow successfully mod-flows from ovs-vtep, there should
be no issue with local set changes either
using add-flow
>
>
>>
>> def add_lbinding(self, lbinding):
>> --
>> 1.9.1
>>
>> _______________________________________________
>> dev mailing list
>> dev@openvswitch.org
>> http://openvswitch.org/mailman/listinfo/dev
>>
>_______________________________________________
>dev mailing list
>dev@openvswitch.org
>http://openvswitch.org/mailman/listinfo/dev
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
