[ovs-dev] [PATCH v2 1/3] ofproto-dpif-sflow: propagate actions within clone

[Zoltán Balogh]

> On Thu, Dec 14, 2017 at 09:56:19AM +0100, Zoltan Balogh wrote:
> > By avoiding Tx recirculation and embracing tnl_push action within clone,
> > the tunnel metadata is not propagated. Unless clone action is handled in
> > the dpif_sflow_read_actions() function as well. This commit resolves
> > this issue.
> > In addition, some sflow data needs to be stored and restored in
> > ofproto-dpif-xlate when native_tunnel_output() is invoked. Otherwise the
> > output action of underlay bridge is getting counted as well if sFlow is
> > set on the overlay bridge.
> >
> > Signed-off-by: Zoltan Balogh <zoltan.balogh at ericsson.com>
> 
> Thanks for working on bug fixes.
> 
> I don't yet understand the first problem that you mention.  Could you
> give an example of something that does not work properly without the
> patch, and how the patch fixes it?

Hi Ben,

As a performance improvement, an "avoid Tx recirculaion" patch was introduced 
in OVS 2.8:

    commit 7c12dfc527a5f6e35eb47494d6284d5a7dbc352c
    tunneling: Avoid datapath-recirc by combining recirc actions at xlate.

With this patch, instead of having two datapath flows (one for over- and one
for underlay bridge) there is only a single combined flow in datapath when
sending packet to native tunnel.
This single flow has a clone() action, which holds the actions from the 
underlay bridge.

Now, when configuring sflow in the overlay bridge, tunnel metadata is not
processed by OVS when reading sflow, because push tunnel action is embedded 
within the clone action. This is fixed by the patch.

> 
> It sounds like this patch addresses two independent bugs.  Could it be
> broken into two patches?

The second bug is connected to the "avoid Tx recirculation" patch as well.
Without fixing this second bug, the number of output ports provided by sflow
is not correct. 

Both are side-effects of the performance improvement patch mentioned above and
both affect sflow handling. That's why I put them into a single patch. 
Furthermore, fixing the one would introduce modifications which should be
removed when fixing the other one later.
By looking at the patch again, I see there these leftover modifications... 
I need to fix it.

Best regards,
Zoltan

> 
> Thanks,
> 
> Ben.