[ovs-dev] [PATCH v5] tunneling: Avoid recirculation on datapath by computing the recirculate actions at translate time.

[Joe Stringer]

On 26 May 2017 at 10:42, Chandran, Sugesh <sugesh.chandran at intel.com> wrote:
>
>
> Regards
> _Sugesh
>
>
>> -----Original Message-----
>> From: Zoltán Balogh [mailto:zoltan.balogh at ericsson.com]
>> Sent: Friday, May 26, 2017 3:01 PM
>> To: Joe Stringer <joe at ovn.org>
>> Cc: Chandran, Sugesh <sugesh.chandran at intel.com>; Andy Zhou
>> <azhou at ovn.org>; William Tu <u9012063 at gmail.com>;
>> gvrose8192 at gmail.com; ovs dev <dev at openvswitch.org>; Ben Pfaff
>> <blp at ovn.org>; Jan Scheurich <jan.scheurich at ericsson.com>; Gray, Mark D
>> <mark.d.gray at intel.com>
>> Subject: RE: [ovs-dev] [PATCH v5] tunneling: Avoid recirculation on datapath
>> by computing the recirculate actions at translate time.
>>
>>
>> Hi Joe,
>>
>> > Backing up a bit for context, the stats attribution goes roughly like this:
>> > * First upcall, handler thread calls through the translate code with a
>> > packet. The resubmit_stats are derived from that packet. This goes
>> > through xlate_actions().
>> > * First dump of flow from revalidator thread fetches the flow and runs
>> > the same xlate_actions() with whatever stats it has (may be zero).
>> > This time, whenever stats attribution or side effects occur, an
>> > xlate_cache entry is generated.
>> > * Second and subsequent dumps of flows fetches the flow and shortcuts
>> > the xlate_actions() by using the xlate_cache instead - ie a call to
>> > xlate_push_stats().
>> >
>> > So, in the same place where the resubmit_stats is manipulated, you
>> > would also need to generate a new XC entry which would manipulate the
>> > stats - this would be a 'side-effect'. I'd imagine that prior to the
>> > full output translation there would be a XC_TRUNCATE(truncated_size)
>> > then afterwards there would be an XC_TRUNCATE_RESET(). Or it could be
>> > just XC_SET_SIZE(...) where 0 is reset and non-zero is a truncate
>> > size. In the implementation/execution in xlate_push_stats() when
>> > performing XC_TRUNCATE you would need to store the original push_stats
>> > size somewhere, then calculate a new 'n_bytes' based on the number of
>> > packets and existing bytes*. For XC_TRUNCATE_RESET(), it would restore
>> > the original push_stats size.
>>
>>  Thank you for the explanation.
>>
>> > * Hmm, I'm not sure the calculation will be 100% here. Let's say there
>> > were 3 packets hit the flow, 50B, 200B, 300B. If output(max_len=100)
>> > was executed, then we don't know how many of the packets were
>> > truncated. The maximum number of bytes that could be transmitted is
>> > 300, but the actual number was 250. We could divide the n_bytes by
>> > n_packets, subtract the max_len and then multiply back up by the
>> > number of packets, which works for this case assuming floating point
>> > arithmetic but is slightly off if using integer math..
>>
>> I don't think, that would be the proper way of calculating n_bytes. Let's say
>> we have 3 packets with 50B, 200B, 200B and max_len=100. The output should
>> be 50 + 100 + 100 = 250B.
>> Following the instructions above we will get
>> [(50 + 200 + 200) / 3 - 100 ] * 3 = [450 / 3 - 100 ] * 3 = 50 * 3 = 150B
>>
>> Any other idea how to calculate the truncated size with xlate cache?
>> Or maybe I did not understand your calculation.
> [Sugesh] Since we have this issue with the trunc action,
> How about limit the combine action only for those tunnels that don’t have any post trunc action.
> If there is a trunc action, Create two separate rules normally as now.
> Is there any other action that would be considered as exception like this?

There's no need for different rules.

Translation of truncate+output is done from
xlate_output_trunc_action(), which calls the common
xlate_output_action() translation after setting up the packet max_len.
Perhaps this path just needs to keep the recirculation, while the
common xlate_output_action() path should be capable of doing this
'combined' (patch-port-style) translation.

I think your last question is effectively asking 'who calls
xlate_output_action'. I see output_reg, output_trunc enqueue, bundle,
and regular output. I think that truncate is the only 'special' case
from this perspective.