[ovs-dev,3/3] ofproto-dpif-xlate.c: Include controller traffic for NetFlow.

The code previously did not include packets forwarded to the controller
in NetFlow, as it considered this control traffic.  That is debatable for
deployments where the first packet of every flow is sent to the
controller for a forwarding decision that may eventually be executed on
the switch.

However, we are starting to send more traffic to local controllers for
non-forwarding purposes such as logging.  These packets are already
being forwarded and only copies are being sent to the controller, so not
accounting for them will incorrectly under-report NetFlow statistics.

Signed-off-by: Justin Pettit <jpettit@ovn.org>
---
 ofproto/ofproto-dpif-xlate.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

> On Mar 22, 2017, at 11:26 PM, Justin Pettit <jpettit@ovn.org> wrote:
> 
> The code previously did not include packets forwarded to the controller
> in NetFlow, as it considered this control traffic.  That is debatable for
> deployments where the first packet of every flow is sent to the
> controller for a forwarding decision that may eventually be executed on
> the switch.
> 

This argues for including controller packets in NetFlow...

> However, we are starting to send more traffic to local controllers for
> non-forwarding purposes such as logging.  These packets are already
> being forwarded and only copies are being sent to the controller,

and this seems to be arguing for the status quo (why should logging copies be included in NetFlow?)...

> so not
> accounting for them will incorrectly under-report NetFlow statistics.
> 

But this then concludes that we must account them anyway.

Maybe you mean that even though a copy of the packet is being sent to controller (maybe for logging purposes), we should still NetFlow the original (upcalled) packet. If so:

Acked-by: Jarno Rajahalme <jarno@ovn.org>


> Signed-off-by: Justin Pettit <jpettit@ovn.org>
> ---
> ofproto/ofproto-dpif-xlate.c | 8 ++------
> 1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
> index 7df92e58dcf5..6b7a4fe51072 100644
> --- a/ofproto/ofproto-dpif-xlate.c
> +++ b/ofproto/ofproto-dpif-xlate.c
> @@ -6464,12 +6464,8 @@ xlate_actions(struct xlate_in *xin, struct xlate_out *xout)
>         ctx.xout->slow |= SLOW_ACTION;
>     }
> 
> -    /* Do netflow only for packets on initial reception, that are not sent to
> -     * the controller.  We consider packets sent to the controller to be part
> -     * of the control plane rather than the data plane. */
> -    if (!xin->frozen_state
> -        && xbridge->netflow
> -        && !(xout->slow & SLOW_CONTROLLER)) {
> +    /* Update NetFlow for non-frozen traffic. */
> +    if (xbridge->netflow && !xin->frozen_state) {
>         if (ctx.xin->resubmit_stats) {
>             netflow_flow_update(xbridge->netflow, flow,
>                                 ctx.nf_output_iface,
> -- 
> 2.7.4
> 
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev

> On Mar 23, 2017, at 10:03 AM, Jarno Rajahalme <jarno@ovn.org> wrote:
> 
> 
>> On Mar 22, 2017, at 11:26 PM, Justin Pettit <jpettit@ovn.org> wrote:
>> 
>> The code previously did not include packets forwarded to the controller
>> in NetFlow, as it considered this control traffic.  That is debatable for
>> deployments where the first packet of every flow is sent to the
>> controller for a forwarding decision that may eventually be executed on
>> the switch.
>> 
> 
> This argues for including controller packets in NetFlow...
> 
>> However, we are starting to send more traffic to local controllers for
>> non-forwarding purposes such as logging.  These packets are already
>> being forwarded and only copies are being sent to the controller,
> 
> and this seems to be arguing for the status quo (why should logging copies be included in NetFlow?)...
> 
>> so not
>> accounting for them will incorrectly under-report NetFlow statistics.
>> 
> 
> But this then concludes that we must account them anyway.
> 
> Maybe you mean that even though a copy of the packet is being sent to controller (maybe for logging purposes), we should still NetFlow the original (upcalled) packet. If so:
> 
> Acked-by: Jarno Rajahalme <jarno@ovn.org>

Yes, that's what I meant.  I tried to clarify the description a little and pushed the series to master.

Thanks for the review.

--Justin