Re: [PATCH net-next 4/4] net: Add Open vSwitch kernel components.

Hi Justin,

I apologize, I did not intend to demean your work. There
are lots of clever ideas in there and you may have wanted
to put out something in a short period out there.

The most basic IMO is to use netlink if you are doing
it from a programmatic interface. You seem to be doing that
already for other items (eg HTB) in the setup. There are
a few libraries out there you could use but i realize
that they may not match your license requirements.
Maybe you could isolate your netlink code and make it 
standalone based on the license you use and people who 
need that could use it.

The other thing, is you match every flow on the specific
virtual port - this may be design intent but it appears
very inflexible.

cheers,
jamal

On Wed, 2011-11-23 at 18:34 -0800, Justin Pettit wrote:

On Nov 22, 2011, at 5:45 PM, Jamal Hadi Salim wrote:

quoted

BTW, you  _are using some of the actions_ already (the policer for
example to do rate control; no disrespect intended but in a terrible
way). 

Hi, Jamal.  I did the initial implementation of the rate control in Open vSwitch.  
Can you help us out with a few more specifics on the problems you see and how they 
could be improved?

Thanks,

--Justin

On Nov 24, 2011, at 5:25 AM, jamal wrote:

The most basic IMO is to use netlink if you are doing
it from a programmatic interface. You seem to be doing that
already for other items (eg HTB) in the setup. There are
a few libraries out there you could use but i realize
that they may not match your license requirements.
Maybe you could isolate your netlink code and make it 
standalone based on the license you use and people who 
need that could use it.

You're right--calling tc directly through system() is kind of ugly.  That code was written a *long* time ago when we wanted a quick QoS story.  As you mentioned, we use netlink to configure traffic shaping, so we have all the pieces at this point.  I just think no one ever bothered to clean up that little wart in userspace.  I'll put that on my to-do list.  Obviously, this doesn't affect the kernel portions.  Thanks for bringing it to our attention.

The other thing, is you match every flow on the specific
virtual port - this may be design intent but it appears
very inflexible.

We encourage users to use shaping, since it generally provides better results (and we do expose per-flow granularity there).  As a result, we haven't seen a need to improve support for policing.

--Justin