Hi Zoltan,

Yes, we disallow truncate followed by patch port as output on purpose.
The reason is that truncate action sets the packet's new length in its
metadata instead of immediately change the size. Actual change of the
packet size happens when we see the output action.

Carrying this un-applied truncate metadata to another bridge
complicates many cases. Ex: what if we truncate, send to patch port,
and the other bridge does broadcast? Do each of the port gets
truncated packets?
Previous discussion:
https://patchwork.ozlabs.org/patch/631972/

I applied your patch and it passes my testcase (posted in previous
email, which makes sure the outer header is matched). Now I see the
issue you mentioned that flow stats are not correct on the underlay
bridge. I'm still trying to figure out the reason.

Thanks.
William

On Mon, May 8, 2017 at 4:15 AM, Zoltán Balogh
<zoltan.bal...@ericsson.com> wrote:
> Hi,
>
> I looked into the code of truncate, I saw that patch ports are not handled.
> On the other hand I saw that "Avoid recirculation" commit should not be 
> affected by this fact. I verified that packets are truncated correctly with 
> my last patch I sent you before, but flow stats are not correct on the 
> underlay bridge.
>
> Could you confirm this, please?
>
> Best regards,
> Zoltan
>
>> -----Original Message-----
>> From: Zoltán Balogh
>> Sent: Sunday, May 07, 2017 9:16 PM
>> To: 'Joe Stringer' <j...@ovn.org>; William Tu <u9012...@gmail.com>
>> Cc: Sugesh Chandran <sugesh.chand...@intel.com>; ovs dev 
>> <d...@openvswitch.org>; Ben Pfaff <b...@ovn.org>; Andy Zhou
>> <az...@ovn.org>; Jan Scheurich <jan.scheur...@ericsson.com>
>> Subject: RE: [ovs-dev] [PATCH v5] tunneling: Avoid recirculation on datapath 
>> by computing the recirculate actions at
>> translate time.
>>
>> > >> I have a patch that fixes tunneling over patch ports. The 14th 
>> > >> system-userspace
>> > >> test still does fail, but now the packet size in the dump flow remains 
>> > >> 242.
>> > >>
>> > >> ./system-traffic.at:554: ovs-ofctl dump-flows br0 | grep "in_port=2" | 
>> > >> sed -n 's/.*\(n\_bytes=[0-9]*\).*/\1/p'
>> > >> ./system-traffic.at:558: ovs-ofctl dump-flows br-underlay | grep 
>> > >> "in_port=LOCAL" | sed -n 's/.*\(n\_bytes=[0-
>> > 9]*\).*/\1/p'
>> > >> --- -   2017-05-05 19:52:28.987111424 +0200
>> > >> +++ 
>> > >> /home/ezolbal/work/general_L3_tunneling/ovs/tests/system-userspace-testsuite.dir/at-groups/14/stdout
>> > 2017-05-05 19:52:28.983508027 +0200
>> > >> @@ -1,2 +1,2 @@
>> > >> -n_bytes=138
>> > >> +n_bytes=242
>> > >>
>> > >> I'm little bit lost with flow statistics. Could you have a look at the 
>> > >> patch
>> > >> below, and help me to find out if it's only a statistics bug, please?
>> > >
>> > > Ah, so the more interesting part of that test is that it also
>> > > truncates the packet during output to the tunnel. So the tunnel should
>> > > only see a 100B packet (encapped within GRE, so 138B). Commit
>> > > aaca4fe0ce9e ("ofp-actions: Add truncate action.") was where this
>> > > functionality was originally introduced, perhaps you can look at that
>> > > to determine how the truncation should be applied in this case?
>> >
>> > Looking again, I think this also states that regardless of the
>> > truncate, the second bridge should attribute stats for the encapped
>> > packet, so even if there was no truncation it should be more than
>> > 242B. When it comes to applying geneve TLVs as well, I'd expect this
>> > to be calculated correctly.
>>
>> Hi Joe,
>> Hi William,
>>
>> I was thinking about the "Avoid recirculation" code created by Sugesh and 
>> myself. It is based on the code patch
>> ports do use.
>> So I reverted our "Avoid recirculation" commit on master branch and tried to 
>> truncate packets on a patch port.
>> I used the setup below.
>>
>>
>>       192.168.10.10         192.168.10.20
>>       ns0                   ns1
>>        |                     |
>>        | br0-ns0             | br1-ns1
>>     +--o-------+          +--o-------+
>>     |          |          |          |
>>     |    br0   |          |    br1   |
>>     |          |          |          |
>>     +--o----o--+          +--o----o--+
>>  veth0 |    | patch0  patch1 |    | veth1
>>        |    +----------------+    |
>>        |                          |
>>        +--------------------------+
>>
>>
>> I attached two namespaces (ns0, ns1) to two netdev bridges (br0, br1), then 
>> I connected the bridges over veth and
>> patch ports.
>>
>>   netdev@ovs-netdev: hit:915736 missed:28
>>           br0:
>>                   br0 65534/1: (tap)
>>                   br0-ns0 1/3: (system)
>>                   patch0 2/none: (patch: peer=patch1)
>>                   veth0 3/5: (system)
>>           br1:
>>                   br1 65534/2: (tap)
>>                   br1-ns1 1/4: (system)
>>                   patch1 2/none: (patch: peer=patch0)
>>                   veth1 3/6: (system)
>>
>> When I added flow rules to forward and truncate packets over veth ports, the 
>> ping from ns0 to ns1 did work.
>>
>> $ ovs-ofctl del-flows br0
>> $ ovs-ofctl del-flows br1
>> $ ovs-ofctl add-flow br0 in_port=1,action=3
>> $ ovs-ofctl add-flow br0 in_port=3,actions=1
>> $ ovs-ofctl add-flow br1 in_port=3,actions=1
>> $ ovs-ofctl add-flow br1 in_port=1,actions=output\(port=3,max_len=200\)
>>
>> $ ip netns exec ns0 ping 192.168.10.20
>>
>>   flow-dump from non-dpdk interfaces:
>>   recirc_id(0),in_port(3),eth_type(0x0800),ipv4(frag=no), packets:180, 
>> bytes:17640, used:0.610s, actions:5
>>   recirc_id(0),in_port(5),eth_type(0x0800),ipv4(frag=no), packets:24, 
>> bytes:2352, used:0.610s, actions:3
>>   recirc_id(0),in_port(4),eth_type(0x0800),ipv4(frag=no), packets:178, 
>> bytes:17444, used:0.610s,
>> actions:trunc(200),6
>>   recirc_id(0),in_port(6),eth_type(0x0800),ipv4(frag=no), packets:25, 
>> bytes:2450, used:0.610s, actions:4
>>
>> When I added flow rules to forward and truncate packets over patch ports, 
>> then ping reply packets did not arrive.
>>
>> $ ovs-ofctl del-flows br0
>> $ ovs-ofctl del-flows br1
>> $ ovs-ofctl add-flow br0 in_port=1,action=2
>> $ ovs-ofctl add-flow br0 in_port=2,actions=1
>> $ ovs-ofctl add-flow br1 in_port=2,actions=1
>> $ ovs-ofctl add-flow br1 in_port=1,actions=output\(port=2,max_len=200\)
>>
>> $ ip netns exec ns0 ping 192.168.10.20
>>
>>   flow-dump from non-dpdk interfaces:
>>   recirc_id(0),in_port(3),eth_type(0x0800),ipv4(frag=no), packets:132, 
>> bytes:12936, used:0.122s, actions:4
>>   recirc_id(0),in_port(4),eth_type(0x0800),ipv4(frag=no), packets:131, 
>> bytes:12838, used:0.121s, actions:drop
>>
>> It seems that truncate on patch ports does not work. Since our "Avoid 
>> recirculation" code reuses the code which
>> handles patch ports, my assumption is that we have to solve two problems.
>>   1) get truncate to work on patch ports.
>>   2) then our "Avoid recirculation" code could reuse the working patch port 
>> code and update flow and base_flow data
>> properly before tunnel header is pushed during translation.
>>
>> What do you think?
>>
>> Best regards,
>> Zoltan
>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to