[ovs-dev] [PATCH v8 1/1] Avoid dp_hash recirculation for balance-tcp bond selection mode
Ben Pfaff
blp at ovn.org
Tue Dec 17 19:23:29 UTC 2019
Thanks for the patch!
"sparse" reports some type errors:
../ofproto/bond.c:357:30: error: incorrect type in assignment (different base types)
../ofproto/bond.c:357:30: expected unsigned int
../ofproto/bond.c:357:30: got restricted ofp_port_t [usertype] ofp_port
../ofproto/ofproto-dpif.c:3454:56: error: incorrect type in argument 2 (different base types)
../ofproto/ofproto-dpif.c:3454:56: expected restricted ofp_port_t [usertype]
../ofproto/ofproto-dpif.c:3454:56: got unsigned int [usertype]
../ofproto/ofproto-dpif.c:3453:31: error: incorrect type in assignment (different base types)
../ofproto/ofproto-dpif.c:3453:31: expected unsigned int [usertype]
../ofproto/ofproto-dpif.c:3453:31: got restricted odp_port_t
../ofproto/ofproto-dpif-xlate.c:4208:39: error: incorrect type in argument 3 (different base types)
../ofproto/ofproto-dpif-xlate.c:4208:39: expected restricted odp_port_t [usertype] value
../ofproto/ofproto-dpif-xlate.c:4208:39: got unsigned int const [usertype] recirc_id
I took a look. The underlying issue is that code here mixes integers,
ofp_port_t, and odp_port_t. OVS uses "sparse" annotations to keep these
from being confused, since they are different in important ways. I
spent some time working through the types here and appended a patch that
fixes them up. I also made a bunch of style updates throughout the
code, which might obscure the relevant changes a bit; my apologies.
It seems odd that dpif_bond_*() succeed if the dpif doesn't support
them. Shouldn't they return an error by default, instead of 0?
Is there a reason to allow users to turn this off? What is the downside
of enabling it? Why is it disabled by default? In general, OVS should
optimize itself to the extent it can rather than relying on a
knowledgeable user to tweak it. If it's necessary to make it
configurable, then the documentation (in vswitch.xml) should explain why
one would want to turn it on or off and what the default is.
This introduces a new capabilities flag, which should be documented with
the other capabilities in vswitch.xml.
I'm appending a suggested diff to fold into your patch.
I did not do a full technical review for correctness. I'll do that on
the next revision.
Thanks,
Ben.
-8<--------------------------cut here-------------------------->8--
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 9a2befdcbc02..fa9ff73a4101 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -604,7 +604,7 @@ struct tx_port {
/* Contained by struct tx_bond 'slave_buckets' */
struct slave_entry {
- uint32_t slave_id;
+ odp_port_t slave_id;
atomic_ullong n_packets;
atomic_ullong n_bytes;
};
@@ -1427,12 +1427,10 @@ dpif_netdev_dp_bond_show(struct unixctl_conn *conn, int argc,
const char *argv[], void *aux OVS_UNUSED)
{
struct ds reply = DS_EMPTY_INITIALIZER;
- struct dp_netdev *dp = NULL;
- uint32_t bucket;
- struct tx_bond *dp_bond_entry = NULL;
ovs_mutex_lock(&dp_netdev_mutex);
+ struct dp_netdev *dp = NULL;
if (argc == 2) {
dp = shash_find_data(&dp_netdevs, argv[1]);
} else if (shash_count(&dp_netdevs) == 1) {
@@ -1446,10 +1444,12 @@ dpif_netdev_dp_bond_show(struct unixctl_conn *conn, int argc,
return;
}
ds_put_cstr(&reply, "\nBonds:\n");
+
+ struct tx_bond *dp_bond_entry;
HMAP_FOR_EACH (dp_bond_entry, node, &dp->bonds) {
ds_put_format(&reply, "\tbond-id %u :\n",
dp_bond_entry->bond_id);
- for (bucket = 0; bucket < BOND_BUCKETS; bucket++) {
+ for (int bucket = 0; bucket < BOND_BUCKETS; bucket++) {
ds_put_format(&reply, "\t\tbucket %u - slave %u \n",
bucket,
dp_bond_entry->slave_buckets[bucket].slave_id);
@@ -1735,7 +1735,6 @@ dp_netdev_free(struct dp_netdev *dp)
OVS_REQUIRES(dp_netdev_mutex)
{
struct dp_netdev_port *port, *next;
- struct tx_bond *bond, *next_bond;
shash_find_and_delete(&dp_netdevs, dp->name);
@@ -1746,6 +1745,7 @@ dp_netdev_free(struct dp_netdev *dp)
ovs_mutex_unlock(&dp->port_mutex);
ovs_mutex_lock(&dp->bond_mutex);
+ struct tx_bond *bond, *next_bond;
HMAP_FOR_EACH_SAFE (bond, next_bond, node, &dp->bonds) {
hmap_remove(&dp->bonds, &bond->node);
free(bond);
@@ -4933,11 +4933,10 @@ pmd_remove_stale_bonds(struct dp_netdev *dp,
OVS_EXCLUDED(pmd->bond_mutex)
OVS_EXCLUDED(dp->bond_mutex)
{
- struct tx_bond *tx, *tx_next;
-
ovs_mutex_lock(&dp->bond_mutex);
ovs_mutex_lock(&pmd->bond_mutex);
+ struct tx_bond *tx, *tx_next;
HMAP_FOR_EACH_SAFE (tx, tx_next, node, &pmd->tx_bonds) {
if (!tx_bond_lookup(&dp->bonds, tx->bond_id)) {
dp_netdev_del_bond_tx_from_pmd(pmd, tx);
@@ -4958,7 +4957,6 @@ reconfigure_datapath(struct dp_netdev *dp)
struct hmapx busy_threads = HMAPX_INITIALIZER(&busy_threads);
struct dp_netdev_pmd_thread *pmd;
struct dp_netdev_port *port;
- struct tx_bond *bond;
int wanted_txqs;
dp->last_reconfigure_seq = seq_read(dp->reconfigure_seq);
@@ -5119,6 +5117,7 @@ reconfigure_datapath(struct dp_netdev *dp)
CMAP_FOR_EACH (pmd, node, &dp->poll_threads) {
ovs_mutex_lock(&pmd->bond_mutex);
if (hmap_count(&pmd->poll_list) || pmd->core_id == NON_PMD_CORE_ID) {
+ struct tx_bond *bond;
HMAP_FOR_EACH (bond, node, &dp->bonds) {
dp_netdev_add_bond_tx_to_pmd(pmd, bond);
}
@@ -5577,13 +5576,11 @@ static void
pmd_load_cached_bonds(struct dp_netdev_pmd_thread *pmd)
OVS_REQUIRES(pmd->bond_mutex)
{
- struct tx_bond *tx_bond, *tx_bond_cached;
-
pmd_free_cached_bonds(pmd);
hmap_shrink(&pmd->bond_cache);
+ struct tx_bond *tx_bond, *tx_bond_cached;
HMAP_FOR_EACH (tx_bond, node, &pmd->tx_bonds) {
- uint32_t bucket = 0;
/* Check if bond already exist on pmd. */
tx_bond_cached = tx_bond_lookup(&pmd->bond_cache, tx_bond->bond_id);
@@ -5594,12 +5591,12 @@ pmd_load_cached_bonds(struct dp_netdev_pmd_thread *pmd)
hash_bond_id(tx_bond_cached->bond_id));
} else {
/* Update the slave-map. */
- for (bucket = 0; bucket <= BOND_MASK; bucket++) {
+ for (int bucket = 0; bucket <= BOND_MASK; bucket++) {
tx_bond_cached->slave_buckets[bucket].slave_id =
tx_bond->slave_buckets[bucket].slave_id;
}
}
- VLOG_DBG("Caching bond-id %d pmd %d\n",
+ VLOG_DBG("Caching bond-id %d pmd %d",
tx_bond_cached->bond_id, pmd->core_id);
}
}
@@ -6432,14 +6429,11 @@ dp_netdev_add_bond_tx_to_pmd(struct dp_netdev_pmd_thread *pmd,
struct tx_bond *bond)
OVS_REQUIRES(pmd->bond_mutex)
{
- struct tx_bond *tx;
- uint32_t i;
bool reload = false;
-
- tx = tx_bond_lookup(&pmd->tx_bonds, bond->bond_id);
+ struct tx_bond *tx = tx_bond_lookup(&pmd->tx_bonds, bond->bond_id);
if (tx) {
/* Check if mapping is changed. */
- for (i = 0; i <= BOND_MASK; i++) {
+ for (int i = 0; i <= BOND_MASK; i++) {
if (bond->slave_buckets[i].slave_id !=
tx->slave_buckets[i].slave_id) {
/* Mapping is modified. Reload pmd bond cache again. */
@@ -6453,7 +6447,7 @@ dp_netdev_add_bond_tx_to_pmd(struct dp_netdev_pmd_thread *pmd,
hmap_insert(&pmd->tx_bonds, &tx->node, hash_bond_id(bond->bond_id));
reload = true;
}
- if (reload == true) {
+ if (reload) {
pmd->need_reload = true;
}
}
@@ -7296,49 +7290,89 @@ dp_execute_userspace_action(struct dp_netdev_pmd_thread *pmd,
}
}
-static int
+static bool
dp_execute_output_action(struct dp_netdev_pmd_thread *pmd,
struct dp_packet_batch *packets_,
- bool should_steal,
- odp_port_t port_no)
+ bool should_steal, odp_port_t port_no)
{
- struct tx_port *p;
- p = pmd_send_port_cache_lookup(pmd, port_no);
- if (OVS_LIKELY(p)) {
- struct dp_packet *packet;
- struct dp_packet_batch out;
- if (!should_steal) {
- dp_packet_batch_clone(&out, packets_);
- dp_packet_batch_reset_cutlen(packets_);
- packets_ = &out;
- }
- dp_packet_batch_apply_cutlen(packets_);
+ struct tx_port *p = pmd_send_port_cache_lookup(pmd, port_no);
+ if (!OVS_LIKELY(p)) {
+ return false;
+ }
+
+ struct dp_packet_batch out;
+ if (!should_steal) {
+ dp_packet_batch_clone(&out, packets_);
+ dp_packet_batch_reset_cutlen(packets_);
+ packets_ = &out;
+ }
+ dp_packet_batch_apply_cutlen(packets_);
#ifdef DPDK_NETDEV
- if (OVS_UNLIKELY(!dp_packet_batch_is_empty(&p->output_pkts)
- && packets_->packets[0]->source
- != p->output_pkts.packets[0]->source)) {
- /* netdev-dpdk assumes that all packets in a single
- * output batch has the same source. Flush here to
- * avoid memory access issues. */
- dp_netdev_pmd_flush_output_on_port(pmd, p);
- }
+ if (OVS_UNLIKELY(!dp_packet_batch_is_empty(&p->output_pkts)
+ && packets_->packets[0]->source
+ != p->output_pkts.packets[0]->source)) {
+ /* netdev-dpdk assumes that all packets in a single
+ * output batch has the same source. Flush here to
+ * avoid memory access issues. */
+ dp_netdev_pmd_flush_output_on_port(pmd, p);
+ }
#endif
- if (dp_packet_batch_size(&p->output_pkts)
- + dp_packet_batch_size(packets_) > NETDEV_MAX_BURST) {
- /* Flush here to avoid overflow. */
- dp_netdev_pmd_flush_output_on_port(pmd, p);
- }
- if (dp_packet_batch_is_empty(&p->output_pkts)) {
- pmd->n_output_batches++;
- }
- DP_PACKET_BATCH_FOR_EACH (i, packet, packets_) {
- p->output_pkts_rxqs[dp_packet_batch_size(&p->output_pkts)] =
- pmd->ctx.last_rxq;
- dp_packet_batch_add(&p->output_pkts, packet);
+ if (dp_packet_batch_size(&p->output_pkts)
+ + dp_packet_batch_size(packets_) > NETDEV_MAX_BURST) {
+ /* Flush here to avoid overflow. */
+ dp_netdev_pmd_flush_output_on_port(pmd, p);
+ }
+ if (dp_packet_batch_is_empty(&p->output_pkts)) {
+ pmd->n_output_batches++;
+ }
+
+ struct dp_packet *packet;
+ DP_PACKET_BATCH_FOR_EACH (i, packet, packets_) {
+ p->output_pkts_rxqs[dp_packet_batch_size(&p->output_pkts)] =
+ pmd->ctx.last_rxq;
+ dp_packet_batch_add(&p->output_pkts, packet);
+ }
+ return true;
+}
+
+static bool
+dp_execute_lb_output_action(struct dp_netdev_pmd_thread *pmd,
+ struct dp_packet_batch *packets_,
+ bool should_steal, uint32_t bond)
+{
+ struct tx_bond *p_bond = pmd_tx_bond_cache_lookup(pmd, bond);
+ if (!p_bond) {
+ return false;
+ }
+
+ struct dp_packet_batch del_pkts;
+ dp_packet_batch_init(&del_pkts);
+
+ struct dp_packet *packet;
+ DP_PACKET_BATCH_FOR_EACH (i, packet, packets_) {
+ /*
+ * Lookup the bond-hash table using hash to get the slave.
+ */
+ uint32_t hash = dp_packet_get_rss_hash(packet);
+ struct slave_entry *s_entry = &p_bond->slave_buckets[hash & BOND_MASK];
+ odp_port_t bond_member = s_entry->slave_id;
+ uint32_t size = dp_packet_size(packet);
+
+ struct dp_packet_batch output_pkt;
+ dp_packet_batch_init_packet(&output_pkt, packet);
+ if (OVS_LIKELY(dp_execute_output_action(pmd, &output_pkt, should_steal,
+ bond_member))) {
+ /* Update slave stats. */
+ non_atomic_ullong_add(&s_entry->n_packets, 1);
+ non_atomic_ullong_add(&s_entry->n_bytes, size);
+ } else {
+ dp_packet_batch_add(&del_pkts, packet);
}
- return 0;
}
- return -1;
+
+ /* Delete packets that failed OUTPUT action */
+ dp_packet_delete_batch(&del_pkts, should_steal);
+ return true;
}
static void
@@ -7352,59 +7386,21 @@ dp_execute_cb(void *aux_, struct dp_packet_batch *packets_,
struct dp_netdev *dp = pmd->dp;
int type = nl_attr_type(a);
struct tx_port *p;
- int ret;
switch ((enum ovs_action_attr)type) {
case OVS_ACTION_ATTR_OUTPUT:
- ret = dp_execute_output_action(pmd, packets_, should_steal,
- nl_attr_get_odp_port(a));
- if (ret == 0) {
- /* Output action executed successfully. */
+ if (dp_execute_output_action(pmd, packets_, should_steal,
+ nl_attr_get_odp_port(a))) {
return;
}
break;
- case OVS_ACTION_ATTR_LB_OUTPUT: {
- uint32_t bond = nl_attr_get_u32(a);
- uint32_t bond_member;
- uint32_t bucket, hash;
- struct dp_packet_batch del_pkts;
- struct dp_packet_batch output_pkt;
- struct dp_packet *packet;
- struct tx_bond *p_bond;
- struct slave_entry *s_entry;
- uint32_t size;
-
- p_bond = pmd_tx_bond_cache_lookup(pmd, bond);
- dp_packet_batch_init(&del_pkts);
- if (p_bond) {
- DP_PACKET_BATCH_FOR_EACH (i, packet, packets_) {
- /*
- * Lookup the bond-hash table using hash to get the slave.
- */
- hash = dp_packet_get_rss_hash(packet);
- bucket = hash & BOND_MASK;
- s_entry = &p_bond->slave_buckets[bucket];
- bond_member = s_entry->slave_id;
- size = dp_packet_size(packet);
-
- dp_packet_batch_init_packet(&output_pkt, packet);
- ret = dp_execute_output_action(pmd, &output_pkt, should_steal,
- u32_to_odp(bond_member));
- if (OVS_UNLIKELY(ret != 0)) {
- dp_packet_batch_add(&del_pkts, packet);
- } else {
- /* Update slave stats. */
- non_atomic_ullong_add(&s_entry->n_packets, 1);
- non_atomic_ullong_add(&s_entry->n_bytes, size);
- }
- }
- /* Delete packets that failed OUTPUT action */
- dp_packet_delete_batch(&del_pkts, should_steal);
+ case OVS_ACTION_ATTR_LB_OUTPUT:
+ if (dp_execute_lb_output_action(pmd, packets_, should_steal,
+ nl_attr_get_u32(a))) {
return;
}
break;
- }
case OVS_ACTION_ATTR_TUNNEL_PUSH:
if (should_steal) {
@@ -7936,23 +7932,23 @@ dpif_netdev_ipf_dump_done(struct dpif *dpif OVS_UNUSED, void *ipf_dump_ctx)
}
static int
-dpif_netdev_bond_add(struct dpif *dpif, uint32_t bond_id, uint32_t slave_map[])
+dpif_netdev_bond_add(struct dpif *dpif, uint32_t bond_id,
+ odp_port_t slave_map[])
{
struct dp_netdev *dp = get_dp_netdev(dpif);
- struct dp_netdev_pmd_thread *pmd;
- uint32_t bucket;
- struct tx_bond *dp_bond_entry = NULL;
ovs_mutex_lock(&dp->bond_mutex);
/*
* Lookup for the bond. If already exists, just update the slave-map.
* Else create new.
*/
- dp_bond_entry = tx_bond_lookup(&dp->bonds, bond_id);
+ struct tx_bond *dp_bond_entry = tx_bond_lookup(&dp->bonds, bond_id);
if (dp_bond_entry) {
- for (bucket = 0; bucket <= BOND_MASK; bucket++) {
+ for (int bucket = 0; bucket <= BOND_MASK; bucket++) {
dp_bond_entry->slave_buckets[bucket].slave_id = slave_map[bucket];
}
+
+ struct dp_netdev_pmd_thread *pmd;
CMAP_FOR_EACH (pmd, node, &dp->poll_threads) {
ovs_mutex_lock(&pmd->bond_mutex);
dp_netdev_add_bond_tx_to_pmd(pmd, dp_bond_entry);
@@ -7961,12 +7957,14 @@ dpif_netdev_bond_add(struct dpif *dpif, uint32_t bond_id, uint32_t slave_map[])
} else {
struct tx_bond *dp_bond = xzalloc(sizeof *dp_bond);
dp_bond->bond_id = bond_id;
- for (bucket = 0; bucket < BOND_BUCKETS; bucket++) {
+ for (int bucket = 0; bucket < BOND_BUCKETS; bucket++) {
dp_bond->slave_buckets[bucket].slave_id = slave_map[bucket];
}
hmap_insert(&dp->bonds, &dp_bond->node,
hash_bond_id(dp_bond->bond_id));
+
/* Insert the bond map in all pmds. */
+ struct dp_netdev_pmd_thread *pmd;
CMAP_FOR_EACH (pmd, node, &dp->poll_threads) {
ovs_mutex_lock(&pmd->bond_mutex);
dp_netdev_add_bond_tx_to_pmd(pmd, dp_bond);
@@ -7981,15 +7979,14 @@ static int
dpif_netdev_bond_del(struct dpif *dpif, uint32_t bond_id)
{
struct dp_netdev *dp = get_dp_netdev(dpif);
- struct dp_netdev_pmd_thread *pmd;
- struct tx_bond *dp_bond_entry = NULL;
ovs_mutex_lock(&dp->bond_mutex);
/* Find the bond and delete it if present */
- dp_bond_entry = tx_bond_lookup(&dp->bonds, bond_id);
+ struct tx_bond *dp_bond_entry = tx_bond_lookup(&dp->bonds, bond_id);
if (dp_bond_entry) {
/* Remove the bond map in all pmds. */
+ struct dp_netdev_pmd_thread *pmd;
CMAP_FOR_EACH (pmd, node, &dp->poll_threads) {
ovs_mutex_lock(&pmd->bond_mutex);
dp_netdev_del_bond_tx_from_pmd(pmd, dp_bond_entry);
@@ -8008,24 +8005,22 @@ dpif_netdev_bond_stats_get(struct dpif *dpif, uint32_t bond_id,
uint64_t *n_bytes)
{
struct dp_netdev *dp = get_dp_netdev(dpif);
- struct dp_netdev_pmd_thread *pmd;
- struct tx_bond *dp_bond_entry = NULL;
- struct tx_bond *pmd_bond_entry = NULL;
- uint32_t i;
ovs_mutex_lock(&dp->bond_mutex);
/* Find the bond and retrieve stats if present */
- dp_bond_entry = tx_bond_lookup(&dp->bonds, bond_id);
+ struct tx_bond *dp_bond_entry = tx_bond_lookup(&dp->bonds, bond_id);
if (dp_bond_entry) {
/* Search the bond in all PMDs */
+ struct dp_netdev_pmd_thread *pmd;
CMAP_FOR_EACH (pmd, node, &dp->poll_threads) {
- uint64_t pmd_n_bytes;
ovs_mutex_lock(&pmd->bond_mutex);
- pmd_bond_entry = tx_bond_lookup(&pmd->bond_cache, bond_id);
+ struct tx_bond *pmd_bond_entry
+ = tx_bond_lookup(&pmd->bond_cache, bond_id);
if (pmd_bond_entry) {
/* Read bond stats. */
- for (i = 0;i <= BOND_MASK; i++) {
+ for (int i = 0; i <= BOND_MASK; i++) {
+ uint64_t pmd_n_bytes;
atomic_read_relaxed(
&pmd_bond_entry->slave_buckets[i].n_bytes,
&pmd_n_bytes);
diff --git a/lib/dpif-provider.h b/lib/dpif-provider.h
index c544684ec0c0..82e2149d2a79 100644
--- a/lib/dpif-provider.h
+++ b/lib/dpif-provider.h
@@ -616,7 +616,8 @@ struct dpif_class {
struct ofputil_meter_stats *, uint16_t n_bands);
/* Adds a bond with 'bond_id' and the slave-map to 'dpif'. */
- int (*bond_add)(struct dpif *dpif, uint32_t bond_id, uint32_t slave_map[]);
+ int (*bond_add)(struct dpif *dpif, uint32_t bond_id,
+ odp_port_t slave_map[]);
/* Removes bond identified by 'bond_id' from 'dpif'. */
int (*bond_del)(struct dpif *dpif, uint32_t bond_id);
/* Reads bond stats from 'dpif'. */
diff --git a/lib/dpif.c b/lib/dpif.c
index 2411c2cbeb41..71a752dba7dd 100644
--- a/lib/dpif.c
+++ b/lib/dpif.c
@@ -1990,7 +1990,7 @@ dpif_meter_del(struct dpif *dpif, ofproto_meter_id meter_id,
}
int
-dpif_bond_add(struct dpif *dpif, uint32_t bond_id, uint32_t slave_map[])
+dpif_bond_add(struct dpif *dpif, uint32_t bond_id, odp_port_t slave_map[])
{
int error = 0;
@@ -2013,8 +2013,9 @@ dpif_bond_del(struct dpif *dpif, uint32_t bond_id)
return error;
}
-int dpif_bond_stats_get(struct dpif *dpif, uint32_t bond_id,
- uint64_t *n_bytes)
+int
+dpif_bond_stats_get(struct dpif *dpif, uint32_t bond_id,
+ uint64_t *n_bytes)
{
int error = 0;
diff --git a/lib/dpif.h b/lib/dpif.h
index 82b1901697a3..6907bf19925d 100644
--- a/lib/dpif.h
+++ b/lib/dpif.h
@@ -907,7 +907,7 @@ bool dpif_supports_tnl_push_pop(const struct dpif *);
bool dpif_supports_balance_tcp_opt(const struct dpif *);
-int dpif_bond_add(struct dpif *dpif, uint32_t bond_id, uint32_t slave_map[]);
+int dpif_bond_add(struct dpif *dpif, uint32_t bond_id, odp_port_t slave_map[]);
int dpif_bond_del(struct dpif *dpif, uint32_t bond_id);
int dpif_bond_stats_get(struct dpif *dpif, uint32_t bond_id,
uint64_t *n_bytes);
diff --git a/ofproto/bond.c b/ofproto/bond.c
index df9110e92d18..59fc5c9c43b3 100644
--- a/ofproto/bond.c
+++ b/ofproto/bond.c
@@ -335,7 +335,7 @@ update_recirc_rules__(struct bond *bond, uint32_t bond_recirc_id)
uint64_t ofpacts_stub[128 / 8];
struct ofpbuf ofpacts;
int i;
- uint32_t slave_map[BOND_MASK];
+ ofp_port_t slave_map[BOND_MASK];
ofpbuf_use_stub(&ofpacts, ofpacts_stub, sizeof ofpacts_stub);
@@ -356,7 +356,7 @@ update_recirc_rules__(struct bond *bond, uint32_t bond_recirc_id)
&bond->hash[i].pr_rule);
slave_map[i] = slave->ofp_port;
} else {
- slave_map[i] = -1;
+ slave_map[i] = OFP_PORT_C(-1);
}
}
ofproto_dpif_bundle_add(bond->ofproto, bond->recirc_id, slave_map);
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 13789ac35225..5c1d0b981e06 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -4203,9 +4203,8 @@ compose_output_action__(struct xlate_ctx *ctx, ofp_port_t ofp_port,
*
* Currently support for netdev datapath only.
*/
- nl_msg_put_odp_port(ctx->odp_actions,
- OVS_ACTION_ATTR_LB_OUTPUT,
- xr->recirc_id);
+ nl_msg_put_u32(ctx->odp_actions, OVS_ACTION_ATTR_LB_OUTPUT,
+ xr->recirc_id);
} else {
act_hash = nl_msg_put_unspec_uninit(ctx->odp_actions,
OVS_ACTION_ATTR_HASH,
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 5f9f1889bc32..1ccebcab4438 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -3441,32 +3441,23 @@ bundle_remove(struct ofport *port_)
int
ofproto_dpif_bundle_add(struct ofproto_dpif *ofproto,
- uint32_t bond_id,
- uint32_t slave_map[])
+ uint32_t bond_id, const ofp_port_t slave_map[])
{
- int error;
- uint32_t bucket;
-
/* Convert ofp_port to odp_port */
- for (bucket = 0; bucket < BOND_BUCKETS; bucket++) {
- if (slave_map[bucket] != -1) {
- slave_map[bucket] =
- ofp_port_to_odp_port(ofproto, slave_map[bucket]);
- }
+ odp_port_t odp_map[BOND_BUCKETS];
+ for (int bucket = 0; bucket < BOND_BUCKETS; bucket++) {
+ odp_map[bucket] = (slave_map[bucket] == OFP_PORT_C(-1)
+ ? ODP_PORT_C(-1)
+ : ofp_port_to_odp_port(ofproto, slave_map[bucket]));
}
- error = dpif_bond_add(ofproto->backer->dpif, bond_id, slave_map);
- return error;
+ return dpif_bond_add(ofproto->backer->dpif, bond_id, odp_map);
}
int
-ofproto_dpif_bundle_del(struct ofproto_dpif *ofproto,
- uint32_t bond_id)
+ofproto_dpif_bundle_del(struct ofproto_dpif *ofproto, uint32_t bond_id)
{
- int error;
-
- error = dpif_bond_del(ofproto->backer->dpif, bond_id);
- return error;
+ return dpif_bond_del(ofproto->backer->dpif, bond_id);
}
static void
diff --git a/ofproto/ofproto-dpif.h b/ofproto/ofproto-dpif.h
index 484a6d4b5272..aa46d9d67fe2 100644
--- a/ofproto/ofproto-dpif.h
+++ b/ofproto/ofproto-dpif.h
@@ -378,11 +378,9 @@ int ofproto_dpif_add_internal_flow(struct ofproto_dpif *,
struct rule **rulep);
int ofproto_dpif_delete_internal_flow(struct ofproto_dpif *, struct match *,
int priority);
-int ofproto_dpif_bundle_add(struct ofproto_dpif *,
- uint32_t bond_id,
- uint32_t slave_map[]);
-int ofproto_dpif_bundle_del(struct ofproto_dpif *,
- uint32_t bond_id);
+int ofproto_dpif_bundle_add(struct ofproto_dpif *, uint32_t bond_id,
+ const ofp_port_t slave_map[]);
+int ofproto_dpif_bundle_del(struct ofproto_dpif *, uint32_t bond_id);
bool ovs_native_tunneling_is_on(struct ofproto_dpif *);
More information about the dev
mailing list