[ovs-dev] [PATCH] odp-util: Update ODPUTIL_FLOW_KEY_BYTES for current kernel flow format.

Ben Pfaff blp at nicira.com
Tue May 15 17:29:00 UTC 2012


On Tue, May 15, 2012 at 10:25:22AM -0700, Justin Pettit wrote:
> On May 15, 2012, at 9:58 AM, Ben Pfaff wrote:
> 
> > On Tue, May 15, 2012 at 09:40:46AM -0700, Justin Pettit wrote:
> >> That's quite a packet.  The change looks reasonable to me--and 200
> >> bytes seems adequate to cover that packet.  However, it might be worth
> >> having Jesse double-check that that's the correct encapsulation in the
> >> example
> > 
> > I am always in favor of more checking, especially by careful developers
> > like Jesse, but perhaps I gave an incorrect impression that this value
> > depends on kernel code?  It doesn't.  ODPUTIL_FLOW_KEY_BYTES is the
> > maximum number of bytes that the userspace function
> > odp_flow_key_from_flow() can write to a buffer.
> 
> Okay.  However, I thought that function depended on the number and
> type of attributes that the kernel used to represent a flow.  You and
> Jesse have the best understanding of what that interface is, and since
> this hypothetical flow is complicated, I suggested Jesse may want to
> look at it.  If you're satisfied that it's correct (or I'm
> misunderstanding something), I obviously trust your judgment.

I'd be delighted to have Jesse take a look.  Jesse?

> >> Do you think it's worth having the total be a #define, and then do a
> >> build-time assertion that the total value is less than
> >> ODPUTIL_FLOW_KEY_BYTES?
> > 
> > Can you explain how that would be different?
> 
> Right now, we have a chart that maps out the total number of bytes
> needed to represent the largest flow.  When updating that chart it's
> pretty obvious that the "total" field at the bottom needs to be
> updated.  I feel like we've had issues in the past where "total" was
> updated, but ODPUTIL_FLOW_KEY_BYTES was not.  This would serve as a
> reminder.  It's pretty unlikely to occur, but I was thinking it would
> be a good sanity-check.  We could also mandate a certain amount a
> slack.  However, if you think it's unnecessary, I'm fine not adding
> one, too.

Mostly I don't understand what you envision the code or comment looking
like.  Can you show me?



More information about the dev mailing list