[ovs-dev] [PATCH 2/9] ofproto/trace: Propagate ct_zone in recirculation

Ben Pfaff blp at ovn.org
Tue Oct 31 22:23:03 UTC 2017


On Thu, Aug 31, 2017 at 02:38:35PM -0700, Greg Rose wrote:
> On 08/25/2017 03:51 PM, Yi-Hung Wei wrote:
> >This patch propagates ct_zone when ofproto/trace automatically runs
> >through the recirculation process.
> >
> >Fixes: e6bc8e749381 ("ofproto/trace: Add support for tracing conntrack recirculation")
> >Signed-off-by: Yi-Hung Wei <yihung.wei at gmail.com>
> >---
> >  ofproto/ofproto-dpif-trace.c |  4 +++-
> >  ofproto/ofproto-dpif-trace.h |  3 ++-
> >  ofproto/ofproto-dpif-xlate.c |  7 ++++---
> >  tests/ofproto-dpif.at        | 14 +++++++-------
> >  4 files changed, 16 insertions(+), 12 deletions(-)
> >
> >diff --git a/ofproto/ofproto-dpif-trace.c b/ofproto/ofproto-dpif-trace.c
> >index a45c9cfd9619..c3c929520a2d 100644
> >--- a/ofproto/ofproto-dpif-trace.c
> >+++ b/ofproto/ofproto-dpif-trace.c
> >@@ -91,7 +91,8 @@ oftrace_node_destroy(struct oftrace_node *node)
> >  bool
> >  oftrace_add_recirc_node(struct ovs_list *recirc_queue,
> >                          enum oftrace_recirc_type type, const struct flow *flow,
> >-                        const struct dp_packet *packet, uint32_t recirc_id)
> >+                        const struct dp_packet *packet, uint32_t recirc_id,
> >+                        const uint16_t zone)
> 
> This function is beginning to get a lot of parameters.  As a suggestion perhaps you might create a helper struct
> to contain all the parameters and just pass a pointer.  Personally I start looking for ways to cut down on parameter
> passing when a function gets to 4 or more parameters.  Again - just a personal predilection.
> 
> Otherwise the patch LGTM.
> 
> Reviewed-by: Greg Rose <gvrose8192 at gmail.com>

Thanks Yi-Hung and Greg.

I applied this to master.  Greg, I think that your comment is fair, but
it's not a crisis yet so I'll leave that for a possible later
improvement.


More information about the dev mailing list