On Thu, Sep 04, 2014 at 02:48:37PM +0200, Jiri Pirko wrote: > Wed, Sep 03, 2014 at 06:37:08PM CEST, john.fastab...@gmail.com wrote: > >On 09/03/2014 02:24 AM, Jiri Pirko wrote: > >>Benefit from the possibility to work with flows in switch devices and > >>use the swdev api to offload flow datapath. > >> > >>Signed-off-by: Jiri Pirko <j...@resnulli.us> > >>--- > >> net/openvswitch/Makefile | 3 +- > >> net/openvswitch/datapath.c | 33 ++++++ > >> net/openvswitch/datapath.h | 3 + > >> net/openvswitch/flow_table.c | 1 + > >> net/openvswitch/hw_offload.c | 245 > >> +++++++++++++++++++++++++++++++++++++++++ > >> net/openvswitch/hw_offload.h | 22 ++++ > >> net/openvswitch/vport-netdev.c | 3 + > >> net/openvswitch/vport.h | 2 + > >> 8 files changed, 311 insertions(+), 1 deletion(-) > >> create mode 100644 net/openvswitch/hw_offload.c > >> create mode 100644 net/openvswitch/hw_offload.h > >> > >>diff --git a/net/openvswitch/Makefile b/net/openvswitch/Makefile > >>index 3591cb5..5152437 100644 > >>--- a/net/openvswitch/Makefile > >>+++ b/net/openvswitch/Makefile > >>@@ -13,7 +13,8 @@ openvswitch-y := \ > >> flow_table.o \ > >> vport.o \ > >> vport-internal_dev.o \ > >>- vport-netdev.o > >>+ vport-netdev.o \ > >>+ hw_offload.o > >> > >> ifneq ($(CONFIG_OPENVSWITCH_VXLAN),) > >> openvswitch-y += vport-vxlan.o > >>diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c > >>index 75bb07f..3e43e1d 100644 > >>--- a/net/openvswitch/datapath.c > >>+++ b/net/openvswitch/datapath.c > >>@@ -57,6 +57,7 @@ > >> #include "flow_netlink.h" > >> #include "vport-internal_dev.h" > >> #include "vport-netdev.h" > >>+#include "hw_offload.h" > >> > >> int ovs_net_id __read_mostly; > >> > >>@@ -864,6 +865,9 @@ static int ovs_flow_cmd_new(struct sk_buff *skb, struct > >>genl_info *info) > >> acts = NULL; > >> goto err_unlock_ovs; > >> } > >>+ error = ovs_hw_flow_insert(dp, new_flow); > >>+ if (error) > >>+ pr_warn("failed to insert flow into hw\n"); > > > >This is really close to silently failing. I think we need to > >hard fail here somehow and push it back to userspace as part of > >the reply and ovs_notify. > > Yes, I agree. My plan was to handle this in ovs hw/sw/both netlink attr > implementation.
FWIW I agree that handling it in that way makes sense. In particular I think that "both", where the datapath is allowed to fall back to software is a useful mode to have (its the current implementation, right?). But that it is also good to allow user-space more control. > >Otherwise I don't know how to manage the hardware correctly. Consider > >the hardware table is full. In this case user space will continue to > >add rules and they will be silently discarded. Similarly if user space > >adds a flow/action that can not be supported by the hardware it will > >be silently ignored. > > > >Even if we do careful accounting on resources in user space we could > >still get an ENOMEM error from sw_flow_action_create. > > > >Same comment for the other hw commands flush/remove. > > > >> if (unlikely(reply)) { > >> error = ovs_flow_cmd_fill_info(new_flow, > >>@@ -896,10 +900,18 @@ static int ovs_flow_cmd_new(struct sk_buff *skb, > >>struct genl_info *info) > >> goto err_unlock_ovs; > >> } > >> } > > > > > >[...] > > > > > >Thanks, > >John > > > >-- > >John Fastabend Intel Corporation > _______________________________________________ > dev mailing list > dev@openvswitch.org > http://openvswitch.org/mailman/listinfo/dev > _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev