Date:	Sat, 23 Aug 2014 21:53:31 -0400
From:	Jamal Hadi Salim <jhs@...atatu.com>
To:	Scott Feldman <sfeldma@...ulusnetworks.com>,
	John Fastabend <john.fastabend@...il.com>
CC:	Jiri Pirko <jiri@...nulli.us>, netdev@...r.kernel.org,
	davem@...emloft.net, nhorman@...driver.com, andy@...yhouse.net,
	tgraf@...g.ch, dborkman@...hat.com, ogerlitz@...lanox.com,
	jesse@...ira.com, pshelar@...ira.com, azhou@...ira.com,
	ben@...adent.org.uk, stephen@...workplumber.org,
	jeffrey.t.kirsher@...el.com, vyasevic@...hat.com,
	xiyou.wangcong@...il.com, john.r.fastabend@...el.com,
	edumazet@...gle.com, f.fainelli@...il.com,
	roopa@...ulusnetworks.com, linville@...driver.com,
	dev@...nvswitch.org, jasowang@...hat.com, ebiederm@...ssion.com,
	nicolas.dichtel@...nd.com, ryazanov.s.a@...il.com,
	buytenh@...tstofly.org, aviadr@...lanox.com, nbd@...nwrt.org,
	alexei.starovoitov@...il.com, Neil.Jerram@...aswitch.com,
	ronye@...lanox.com
Subject: Re: [patch net-next RFC 10/12] openvswitch: add support for datapath
 hardware offload

On 08/22/14 18:53, Scott Feldman wrote:

Ok, Scott - now i have looked at the patches on the plane and i am
still not convinced ;->

> The intent is to use openvswitch.ko’s struct sw_flow to program hardware via the
>ndo_swdev_flow_* ops, but otherwise be independent of OVS.  So the upper layer of
>the driver is struct sw_flow and any module above the driver can construct a struct
>sw_flow and push it down via ndo_swdev_flow_*.  So your non-OVS use-case should be
>handled.  OVS is another use-case.  struct sw_flow should not be OVS-aware, but
>rather a generic flow match/action sufficient to offload the data plane to HW.


There is a legitimate case to be made for offloading OVS but *not*
a basis for making it the offload interface.
My suggestion is to make all OVS stuff a separate patchset.
This thing needs to stand alone without OVS and we dont need
to confuse the two.

Having said that:
I believe in starting simple - by solving the basic functions of
L2/3 offload first because those are well understood and fundamental.
There is the simplicity of those network functions and then
need to deal with tons of quarks that surround them....
I think getting that right will help in understanding the issues and
make this interface better. This is where i am going to focus my effort.

Here's my view on flows in the patchset:
What we need is ability to specify different types of classifiers.
But leave L2 and 3 out of that - that should be part of the basic
feature set.
Your 15-tuple classifier should be one of those classifiers.
This is because you *cannot possibly* have a universal classifier.
The tc classifier/action API has got this part right. There is
no ONE flow classifier but rather it has flexibility to add as many
as you want.
IOW:
I should be able to specify a classifier that matches the
definition of the openflow thing you are using. But then i should also
be able to create one based on 32 bit value/masks, one that classifies
strings, one that classifies metadata, my own pigeon observer
classifier etc. And be able to attach them in combinations
to select different things within the packet and act differently.

Lets pick an example of the u32 classifier (or i could pick nftables).
Using your scheme i have to incur penalties to translating u32 to your
classifier and only achieve basic functionality; and now in addition
i cant do 90% of my u32 features. And u32 is very implementable
in hardware.

cheers,
jamal


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Powered by blists - more mailing lists