[ovs-dev] [PATCH v11 5/5] dpif-netdev: Add specialized generic scalar functions

Van Haaren, Harry harry.van.haaren at intel.com
Wed Jul 17 10:35:58 UTC 2019


> -----Original Message-----
> From: Ilya Maximets [mailto:i.maximets at samsung.com]
> Sent: Wednesday, July 17, 2019 11:24 AM
> To: Van Haaren, Harry <harry.van.haaren at intel.com>; dev at openvswitch.org
> Cc: echaudro at redhat.com; malvika.gupta at arm.com; Stokes, Ian
> <ian.stokes at intel.com>
> Subject: Re: [PATCH v11 5/5] dpif-netdev: Add specialized generic scalar
> functions
> 
> On 17.07.2019 13:18, Van Haaren, Harry wrote:

<snip>

> >>> +    if (f) {
> >>> +        VLOG_INFO("Subtable using Generic Optimized for u0 %d, u1 %d\n",
> >>> +                  u0_bits, u1_bits);
> >>
> >> Can we move this message out to 'dpcls_create_subtable' and log the
> >> non-optimized case too?
> >> Also, this needs to be DBG as all other subtable related logs are DBG logs.
> >>
> >> We could just extend the 'Creating subtable' log message to include this
> >> information:
> >>
> >>     VLOG_DBG("Creating %"PRIuSIZE". subtable %p for in_port %d with a %s "
> >>              "lookup function (units: %"PRIu32", %"PRIu32").",
> >>              cmap_count(&cls->subtables_map), subtable, cls->in_port,
> >>              (subtable->lookup_func == dpcls_subtable_lookup_generic)
> >>              ? "generic" : "specialized", unit0, unit1);
> >
> >
> > This might seem a nice idea now, however in future we can plug in other
> optimized
> > flavors of lookups, and then the dpcls_create_subtable() function won't know
> > the details of the implementation. Hence having the log in the
> implementation
> > is the better solution.
> 
> If there will be other implementations we'll have to change the prototype of
> 'dpcls_subtable_generic_probe' and refactor the code around anyway. So,
> changing
> the log message would be a minor thing. Your version of
> 'dpcls_create_subtable'
> already highly depends on the implementation of
> 'dpcls_subtable_generic_probe'.

If we plug in new implementations, they will also have their own probe() function,
so there is no need to modify the existing probe prototype.

The log message is implementation specific (the name and details of the implementation),
so a print at that level (not an abstraction level above it) makes more sense here.

I'll leave this as is for v12, but change to DEBUG as suggested.


More information about the dev mailing list