On Mon, May 8, 2017 at 11:29 AM, Zoltán Balogh
<zoltan.bal...@ericsson.com> wrote:
> Hi William,
>
> The reason of 'incorrect' n_bytes stats could be due to the mechanism truncate
> and tunneling with clone action do work. As you wrote, the packet size is
> changed when the output action is applied.
>
> Let's say, I have the config below:
>
>        ns1
>         |
>      +--o-----+
>      | br-int |
>      +-----o--+
>           gre0
>       LOCAL
>      +-o------+
>      |  br-p  |
>      +-----o--+
>            |
>           ns2
>
> $ ovs-ofctl dump-flows br-int
> NXST_FLOW reply (xid=0x4):
>  cookie=0x0, duration=525.264s, table=0, n_packets=4, n_bytes=4168, 
> idle_age=62, in_port=1 actions=output(port=2,max_len=200)
> $ ovs-ofctl dump-flows br-p
> NXST_FLOW reply (xid=0x4):
>  cookie=0x0, duration=527.255s, table=0, n_packets=5, n_bytes=4210, 
> idle_age=64, in_port=LOCAL actions=dec_ttl,output:3
> $ ovs-vsctl show
> 73ee7b44-e781-4dfd-ad4c-f7790f644000
>     Bridge br-int
>         Port br-int
>             Interface br-int
>                 type: internal
>         Port "br-int-ns1"
>             Interface "br-int-ns1"
>         Port "gre0"
>             Interface "gre0"
>                 type: gre
>                 options: {remote_ip="10.0.0.20"}
>     Bridge br-p
>         Port "br-p-ns2"
>             Interface "br-p-ns2"
>         Port br-p
>             Interface br-p
>                 type: internal
>
> If I send IPCM echo request from ns1 towards ns2 through the GRE tunnel, then
> I get the following datapath flow:
>
> netdev@ovs-netdev: hit:7 missed:11
>         br-int:
>                 br-int 65534/1: (tap)
>                 br-int-ns1 1/3: (system)
>                 gre0 2/5: (gre: remote_ip=10.0.0.20)
>         br-p:
>                 br-p 65534/2: (tap)
>                 br-p-ns2 3/4: (system)
>
> flow-dump from non-dpdk interfaces:
> recirc_id(0),in_port(3),eth_type(0x0800),ipv4(tos=0/0x3,ttl=64,frag=no), 
> packets:0, bytes:0, used:never, actions:trunc(200),clone(tnl_push(tnl_port(5)
> ,header(size=38,type=3,eth(dst=be:a5:83:73:e9:dc,src=36:23:30:b4:b2:48,dl_type=0x0800),ipv4(src=10.0.0.10,dst=10.0.0.20,proto=47,tos=0,ttl=64,frag=0x4
> 000),gre((flags=0x0,proto=0x6558))),out_port(2)),set(ipv4(tos=0/0x3,ttl=63)),4)
>
> The packet arrives on port 3 (br-int-ns1), then cutlen is set via trunc, then
> clone is applied. Clone pushes tunnel header, then tos/ttl is set, finally, 
> the
> packet is output to port 4 (br-p-ns2). And this is the point where packet size
> is changed, at the end of the datapath flow. So, we have a single flow, and
> size of a packet will not be changed while actions are applied except at the
> end of processing via output action.

It may be cleaner if we add a new trunc action for the datapath, say
trunc2  that applies
to all outputs within the clone.

So the translation will look like: clone(trunc2, native tunnel
translation). Would this
approach work?

>
> Without the "Avoid recirculation" patch we have two datapath flows, because 
> the
> packet is recirculated. At the end of the first flow the packet size is 
> changed
> and the packet with modified size enters the OF pipeline again.
>
> What is the reason not to change packet size when truncate action is applied?
>

One of the reasons could be that we introduced trunc before clone. Otherwise, a
clone(trunc2, output:x) is equivalent to trunc, output:x.  Note that
the trunc datapath
action is different than other datapath actions, which usually applies
to all following
actions. Native tunneling may be the first use case that motivates
trunc2, which should
have the normal datapath action behavior.

> Best regards,
> Zoltan
>
>
>> -----Original Message-----
>> From: William Tu [mailto:u9012...@gmail.com]
>> Sent: Monday, May 08, 2017 4:55 PM
>> To: Zoltán Balogh <zoltan.bal...@ericsson.com>
>> Cc: Joe Stringer <j...@ovn.org>; 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.
>>
>> 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