Re: [RFC net-next 2/9] net/switchdev: Introduce hardware offload support

[amirva@gmail.com]

On Mon, Feb 01, 2016 at 10:06:27AM +0100, Jiri Pirko wrote:
> Mon, Feb 01, 2016 at 09:34:38AM CET, amir@vadai.me wrote:
> >Extend the switchdev API with new operations: switchdev_port_flow_add()
> >and switchdev_port_flow_del().
> >It allows the user to add/del a hardware offloaded flow classification
> >and actions.
> >For every new flow object a cookie is supplied. This cookie will be
> >used later on to identify the flow when removed.
> >
> >In order to make the API as flexible as possible, flow_dissector is
> >being used to describe the flow classifier.
> >
> >Every new flow object is consists of a flow_dissector+key+mask to
> >describe the classifier and a switchdev_obj_port_flow_act to describe
> >the actions and their attributes.
> >
> >object is passed to the lower layer driver to be pushed into the
> >hardware.
> >
> >Signed-off-by: Amir Vadai <amir@vadai.me>
> >---
> > include/net/switchdev.h   | 46 ++++++++++++++++++++++++++++++++++++++++++++++
> > net/switchdev/switchdev.c | 33 +++++++++++++++++++++++++++++++++
> > 2 files changed, 79 insertions(+)
> >
> >diff --git a/include/net/switchdev.h b/include/net/switchdev.h
> >index d451122..c5a5681 100644
> >--- a/include/net/switchdev.h
> >+++ b/include/net/switchdev.h
> >@@ -15,6 +15,7 @@
> > #include <linux/notifier.h>
> > #include <linux/list.h>
> > #include <net/ip_fib.h>
> >+#include <net/flow_dissector.h>
> > 
> > #define SWITCHDEV_F_NO_RECURSE		BIT(0)
> > #define SWITCHDEV_F_SKIP_EOPNOTSUPP	BIT(1)
> >@@ -69,6 +70,7 @@ enum switchdev_obj_id {
> > 	SWITCHDEV_OBJ_ID_IPV4_FIB,
> > 	SWITCHDEV_OBJ_ID_PORT_FDB,
> > 	SWITCHDEV_OBJ_ID_PORT_MDB,
> >+	SWITCHDEV_OBJ_ID_PORT_FLOW,
> > };
> > 
> > struct switchdev_obj {
> >@@ -124,6 +126,30 @@ struct switchdev_obj_port_mdb {
> > #define SWITCHDEV_OBJ_PORT_MDB(obj) \
> > 	container_of(obj, struct switchdev_obj_port_mdb, obj)
> > 
> >+/* SWITCHDEV_OBJ_ID_PORT_FLOW */
> >+enum switchdev_obj_port_flow_action {
> >+	SWITCHDEV_OBJ_PORT_FLOW_ACT_DROP = 0,
> >+	SWITCHDEV_OBJ_PORT_FLOW_ACT_MARK = 1,
> >+};
> >+
> >+struct switchdev_obj_port_flow_act {
> >+	u32 actions; /* Bitmap of requested actions */
> >+	u32 mark; /* Value for mark action - if requested */
> 
> This approach is certainly not correct. We need a list of actions here
> instead of bitmap.
This is what I meant in the cover letter by saying:

"2. Serialization of actions will be changed into a list instead of one
    big structure to describe all actions."