[ovs-dev] [PATCH 5/9] Native OpenFlow 1.2 set_field action support.

[Ben Pfaff]

On Thu, Oct 17, 2013 at 03:32:03PM -0700, Jarno Rajahalme wrote:
> 
> On Oct 17, 2013, at 12:37 PM, Ben Pfaff <blp at nicira.com> wrote:
> 
> > On Wed, Oct 16, 2013 at 04:16:07PM -0700, Jarno Rajahalme wrote:
> >> This is a little bit simpler and marginally more efficient, but also makes
> >> following changes easier.
> >> 
> >> Signed-off-by: Jarno Rajahalme <jrajahalme at nicira.com>
> > 
> > I had to read the patch before I understood what it did.  Could you
> > elaborate the change log slightly?
> > 
> > Any particular reason to inline mf_from_id()?  I don't really object, I
> > think, but rationale would be nice.  Also I don't see yet how this is
> > related to the rest of the patch.  Could it be separated?
> 
> When I decided to use the enum instead of a pointer I decided to
> inline the mf_from_id to avoid any extra cost in action
> translation. It seems to me that the compiler should be able to
> completely optimize mf_from_id away in most cases.
> 
> I can easily separate this, no problem.

Thanks.

> On the same note, we have a lot of heavily used single line/trivial
> utility functions that I think would benefit from
> inlining. Alternatively, LTO might be used for the same effect (?),
> but I have no experience with that so far.

Yes, we have tons of candidates and I'm opening to inlining trivial
ones.  I usually don't personally bother until I see a function show up
in a profile.