On Tue, Jul 10, 2018 at 11:59:35AM +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.basavapa...@broadcom.com>
> Co-authored-by: Venkat Duvvuru <venkatkumar.duvv...@broadcom.com>
> Signed-off-by: Venkat Duvvuru <venkatkumar.duvv...@broadcom.com>
> Reviewed-by: Sathya Perla <sathya.pe...@broadcom.com>
Thanks for the patch.
I spent some time adjusting the style to better fit what we usually do
these days in OVS, which puts declarations as close to first use as
practical. I'm appending the incremental that I'd suggest folding in.
However I noticed a potentially serious bug while doing it. Before this
patch, the code looked for output actions and took the dst_port from the
last one it found that was a tunnel. After this patch, it does the same
thing but it also leaks a netdev for every output action other than the
first. I added a "break;" to mitigate the damage but I guess it's not a
correct solution.
Thanks,
Ben.
--8<--------------------------cut here-------------------------->8--
diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
index 5a6d53ad5697..ecc1ea5f4455 100644
--- a/lib/dpif-netlink.c
+++ b/lib/dpif-netlink.c
@@ -2097,16 +2097,12 @@ 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;
@@ -2137,7 +2133,7 @@ parse_flow_put(struct dpif_netlink *dpif, struct
dpif_flow_put *put)
if (nl_attr_type(nla) == OVS_ACTION_ATTR_OUTPUT) {
const struct netdev_tunnel_config *tnl_cfg;
- out_port = nl_attr_get_odp_port(nla);
+ odp_port_t out_port = nl_attr_get_odp_port(nla);
outdev = netdev_ports_get(out_port, dpif_class);
if (!outdev) {
err = EOPNOTSUPP;
@@ -2147,6 +2143,7 @@ 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;
}
+ break;
}
}
@@ -2177,18 +2174,16 @@ parse_flow_put(struct dpif_netlink *dpif, struct
dpif_flow_put *put)
VLOG_DBG("added flow");
} else if (err != EEXIST) {
- 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;
+ struct netdev *oor_netdev = NULL;
+ if (outdev && err == ENOSPC) {
+ oor_netdev = flow_get_tunnel_netdev(&match.flow.tunnel);
+ if (!oor_netdev) {
+ oor_netdev = dev;
+ }
+ oor_netdev->hw_info.oor = true;
}
VLOG_ERR_RL(&rl, "failed to offload flow: %s: %s", ovs_strerror(err),
- (oor_netdev ? oor_netdev->name : dev->name));
+ oor_netdev ? oor_netdev->name : dev->name);
}
out:
diff --git a/lib/flow.c b/lib/flow.c
index 90a1c0a3aa21..c1191e368419 100644
--- a/lib/flow.c
+++ b/lib/flow.c
@@ -3410,11 +3410,7 @@ flow_limit_vlans(int vlan_limit)
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)) {
@@ -3423,10 +3419,11 @@ flow_get_tunnel_netdev(struct flow_tnl *tunnel)
return NULL;
}
+ char iface[IFNAMSIZ];
+ struct in6_addr gw;
if (!ovs_router_lookup(0, &ip6, iface, NULL, &gw)) {
- return (NULL);
+ return NULL;
}
- tunnel_netdev = netdev_from_name(iface);
- return tunnel_netdev;
+ return netdev_from_name(iface);
}
diff --git a/lib/netdev.c b/lib/netdev.c
index b17f0563fb3b..3e66158824e9 100644
--- a/lib/netdev.c
+++ b/lib/netdev.c
@@ -2473,7 +2473,6 @@ netdev_free_custom_stats_counters(struct
netdev_custom_stats *custom_stats)
}
#ifdef __linux__
-
static void
netdev_ports_flow_init(void)
{
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
