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? _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
