| Message ID | 20180706104050.24938-2-sriharsha.basavapatna@broadcom.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Support dynamic rebalancing of offloaded flows | expand |
Bleep bloop. Greetings Sriharsha Basavapatna via dev, I am a robot and I have tried out your patch. Thanks for your contribution. I encountered some error that I wasn't expecting. See the details below. build: /bin/sh ./libtool --tag=CC --mode=compile gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include -I ./lib -I ./lib -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Werror -MT lib/signals.lo -MD -MP -MF $depbase.Tpo -c -o lib/signals.lo lib/signals.c &&\ mv -f $depbase.Tpo $depbase.Plo libtool: compile: gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include -I ./lib -I ./lib -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Werror -MT lib/signals.lo -MD -MP -MF lib/.deps/signals.Tpo -c lib/signals.c -o lib/signals.o depbase=`echo lib/socket-util-unix.lo | sed 's|[^/]*$|.deps/&|;s|\.lo$||'`;\ /bin/sh ./libtool --tag=CC --mode=compile gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include -I ./lib -I ./lib -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Werror -MT lib/socket-util-unix.lo -MD -MP -MF $depbase.Tpo -c -o lib/socket-util-unix.lo lib/socket-util-unix.c &&\ mv -f $depbase.Tpo $depbase.Plo libtool: compile: gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include -I ./lib -I ./lib -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Werror -MT lib/socket-util-unix.lo -MD -MP -MF lib/.deps/socket-util-unix.Tpo -c lib/socket-util-unix.c -o lib/socket-util-unix.o depbase=`echo lib/stream-unix.lo | sed 's|[^/]*$|.deps/&|;s|\.lo$||'`;\ /bin/sh ./libtool --tag=CC --mode=compile gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include -I ./lib -I ./lib -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Werror -MT lib/stream-unix.lo -MD -MP -MF $depbase.Tpo -c -o lib/stream-unix.lo lib/stream-unix.c &&\ mv -f $depbase.Tpo $depbase.Plo libtool: compile: gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include -I ./lib -I ./lib -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Werror -MT lib/stream-unix.lo -MD -MP -MF lib/.deps/stream-unix.Tpo -c lib/stream-unix.c -o lib/stream-unix.o depbase=`echo lib/dpif-netlink.lo | sed 's|[^/]*$|.deps/&|;s|\.lo$||'`;\ /bin/sh ./libtool --tag=CC --mode=compile gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include -I ./lib -I ./lib -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Werror -MT lib/dpif-netlink.lo -MD -MP -MF $depbase.Tpo -c -o lib/dpif-netlink.lo lib/dpif-netlink.c &&\ mv -f $depbase.Tpo $depbase.Plo libtool: compile: gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include -I ./lib -I ./lib -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Werror -MT lib/dpif-netlink.lo -MD -MP -MF lib/.deps/dpif-netlink.Tpo -c lib/dpif-netlink.c -o lib/dpif-netlink.o lib/dpif-netlink.c:1007:1: error: return type defaults to 'int' [-Werror] dpif_netlink_port_add(struct dpif *dpif_, struct netdev *netdev, ^ lib/dpif-netlink.c:1007:1: error: no previous prototype for 'dpif_netlink_port_add' [-Werror=missing-prototypes] cc1: all warnings being treated as errors make[2]: *** [lib/dpif-netlink.lo] Error 1 make[2]: Leaving directory `/var/lib/jenkins/jobs/upstream_build_from_pw/workspace' make[1]: *** [all-recursive] Error 1 make[1]: Leaving directory `/var/lib/jenkins/jobs/upstream_build_from_pw/workspace' make: *** [all] Error 2 Please check this out. If you feel there has been an error, please email aconole@bytheb.org Thanks, 0-day Robot
On Fri, Jul 06, 2018 at 04:10:47PM +0530, Sriharsha Basavapatna via dev wrote: > This is the first patch in the patch-set to support dynamic rebalancing > of offloaded flows. > > The patch detects OOR condition on a netdev port when ENOSPC error is > returned by TC-Flower while adding a flow rule. A new structure is added > to the netdev called "netdev_hw_info", to store OOR related information > required to perform dynamic offload-rebalancing. > > Signed-off-by: Sriharsha Basavapatna <sriharsha.basavapatna@broadcom.com> > Co-authored-by: Venkat Duvvuru <venkatkumar.duvvuru@broadcom.com> > Signed-off-by: Venkat Duvvuru <venkatkumar.duvvuru@broadcom.com> > Reviewed-by: Sathya Perla <sathya.perla@broadcom.com> I see that the robot already pointed out build errors in the first two patches. Please do correct those and post v2. As a trivial additional note, "ovs-dev:" doesn't make sense as a prefix. Please choose a prefix for each patch that reflects the part of OVS that it primarily affects. Thanks, Ben.
diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c index aa9bbd946..e20096116 100644 --- a/lib/dpif-netlink.c +++ b/lib/dpif-netlink.c @@ -1004,7 +1004,6 @@ dpif_netlink_rtnl_port_create_and_add(struct dpif_netlink *dpif, return error; } -static int dpif_netlink_port_add(struct dpif *dpif_, struct netdev *netdev, odp_port_t *port_nop) { @@ -2097,11 +2096,16 @@ parse_flow_put(struct dpif_netlink *dpif, struct dpif_flow_put *put) { static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 20); const struct dpif_class *dpif_class = dpif->dpif.dpif_class; + struct netdev_hw_info *hw_info; struct match match; odp_port_t in_port; + odp_port_t out_port; const struct nlattr *nla; size_t left; struct netdev *dev; + struct netdev *outdev = NULL; + struct netdev *tunnel_netdev = NULL; + struct netdev *oor_netdev = NULL; struct offload_info info; ovs_be16 dst_port = 0; int err; @@ -2131,8 +2135,6 @@ parse_flow_put(struct dpif_netlink *dpif, struct dpif_flow_put *put) NL_ATTR_FOR_EACH(nla, left, put->actions, put->actions_len) { if (nl_attr_type(nla) == OVS_ACTION_ATTR_OUTPUT) { const struct netdev_tunnel_config *tnl_cfg; - struct netdev *outdev; - odp_port_t out_port; out_port = nl_attr_get_odp_port(nla); outdev = netdev_ports_get(out_port, dpif_class); @@ -2144,7 +2146,6 @@ parse_flow_put(struct dpif_netlink *dpif, struct dpif_flow_put *put) if (tnl_cfg && tnl_cfg->dst_port != 0) { dst_port = tnl_cfg->dst_port; } - netdev_close(outdev); } } @@ -2175,7 +2176,18 @@ parse_flow_put(struct dpif_netlink *dpif, struct dpif_flow_put *put) VLOG_DBG("added flow"); } else if (err != EEXIST) { - VLOG_ERR_RL(&rl, "failed to offload flow: %s", ovs_strerror(err)); + if (outdev && dev && (err == ENOSPC)) { + tunnel_netdev = flow_get_tunnel_netdev(&match.flow.tunnel); + if (tunnel_netdev) { + oor_netdev = tunnel_netdev; + } else { + oor_netdev = dev; + } + hw_info = &oor_netdev->hw_info; + hw_info->oor = true; + } + VLOG_ERR_RL(&rl, "failed to offload flow: %s: %s", ovs_strerror(err), + (oor_netdev ? oor_netdev->name : dev->name)); } out: @@ -2196,6 +2208,7 @@ out: } } + netdev_close(outdev); netdev_close(dev); return err; diff --git a/lib/flow.c b/lib/flow.c index 75ca45672..912afc99d 100644 --- a/lib/flow.c +++ b/lib/flow.c @@ -19,6 +19,7 @@ #include <errno.h> #include <inttypes.h> #include <limits.h> +#include <net/if.h> #include <netinet/in.h> #include <netinet/icmp6.h> #include <netinet/ip6.h> @@ -41,6 +42,8 @@ #include "unaligned.h" #include "util.h" #include "openvswitch/nsh.h" +#include "ovs-router.h" +#include "lib/netdev-provider.h" COVERAGE_DEFINE(flow_extract); COVERAGE_DEFINE(miniflow_malloc); @@ -3299,3 +3302,27 @@ flow_limit_vlans(int vlan_limit) flow_vlan_limit = MIN(vlan_limit, FLOW_MAX_VLAN_HEADERS); } } + +struct netdev * +flow_get_tunnel_netdev(struct flow_tnl *tunnel) +{ + struct netdev *tunnel_netdev; + char iface[IFNAMSIZ]; + struct in6_addr ip6; + struct in6_addr gw; + + if (tunnel->ip_src) { + in6_addr_set_mapped_ipv4(&ip6, tunnel->ip_src); + } else if (ipv6_addr_is_set(&tunnel->ipv6_src)) { + ip6 = tunnel->ipv6_src; + } else { + return NULL; + } + + if (!ovs_router_lookup(0, &ip6, iface, NULL, &gw)) { + return (NULL); + } + + tunnel_netdev = netdev_from_name(iface); + return tunnel_netdev; +} diff --git a/lib/flow.h b/lib/flow.h index 5b6585f11..a67abc9c9 100644 --- a/lib/flow.h +++ b/lib/flow.h @@ -73,6 +73,7 @@ void flow_extract(struct dp_packet *, struct flow *); void flow_zero_wildcards(struct flow *, const struct flow_wildcards *); void flow_unwildcard_tp_ports(const struct flow *, struct flow_wildcards *); void flow_get_metadata(const struct flow *, struct match *flow_metadata); +struct netdev *flow_get_tunnel_netdev(struct flow_tnl *tunnel); const char *ct_state_to_string(uint32_t state); uint32_t ct_state_from_string(const char *); diff --git a/lib/netdev-provider.h b/lib/netdev-provider.h index 1a572f5b8..62e05619e 100644 --- a/lib/netdev-provider.h +++ b/lib/netdev-provider.h @@ -35,6 +35,11 @@ extern "C" { struct netdev_tnl_build_header_params; #define NETDEV_NUMA_UNSPEC OVS_NUMA_UNSPEC +/* Offload-capable (HW) netdev information */ +struct netdev_hw_info { + bool oor; /* Out of Offload Resources ? */ +}; + /* A network device (e.g. an Ethernet device). * * Network device implementations may read these members but should not modify @@ -80,6 +85,8 @@ struct netdev { int n_rxq; struct shash_node *node; /* Pointer to element in global map. */ struct ovs_list saved_flags_list; /* Contains "struct netdev_saved_flags". */ + + struct netdev_hw_info hw_info; /* offload-capable netdev info */ }; static inline void diff --git a/lib/netdev.c b/lib/netdev.c index 82ffeb901..b17f0563f 100644 --- a/lib/netdev.c +++ b/lib/netdev.c @@ -415,6 +415,7 @@ netdev_open(const char *name, const char *type, struct netdev **netdevp) netdev->reconfigure_seq = seq_create(); netdev->last_reconfigure_seq = seq_read(netdev->reconfigure_seq); + netdev->hw_info.oor = false; netdev->node = shash_add(&netdev_shash, name, netdev); /* By default enable one tx and rx queue per netdev. */ @@ -2472,6 +2473,7 @@ netdev_free_custom_stats_counters(struct netdev_custom_stats *custom_stats) } #ifdef __linux__ + static void netdev_ports_flow_init(void) {