@@ -4,11 +4,6 @@
** New OVN logical actions
-*** rate_limit
-
-TCP/IP stacks typically limit the rate at which ARPs are sent, e.g. to
-one per second for a given target. We might need to do this too.
-
*** icmp4 { action... }
Generates an ICMPv4 packet based on the current IPv4 packet and
@@ -61,6 +56,9 @@ using ARP.
From casual observation, Linux appears to generate at most one ARP per
second per destination.
+This might be supported by adding a new OVN logical action for
+rate-limiting.
+
*** Tracking queries
It's probably best to only record in the database responses to queries
@@ -73,6 +71,9 @@ into the database.
Something needs to make sure that bindings remain valid and expire
those that become stale.
+One way to do this might be to add some support for time to the
+database server itself.
+
*** Table size limiting.
The table of MAC bindings must not be allowed to grow unreasonably
@@ -363,7 +363,9 @@ put_load(const uint8_t *data, size_t len,
bitwise_one(&sf->mask, sf->field->n_bytes, ofs, n_bits);
}
-/* Adds a flow to table */
+/* Adds an OpenFlow flow to 'flow_table' for each MAC binding in the OVN
+ * southbound database, using 'lports' to resolve logical port names to
+ * numbers. */
static void
add_neighbor_flows(struct controller_ctx *ctx,
const struct lport_index *lports, struct hmap *flow_table)
@@ -17,6 +17,7 @@
#include "pinctrl.h"
+#include "coverage.h"
#include "dirs.h"
#include "dp-packet.h"
#include "lport.h"
@@ -44,18 +45,23 @@ static struct rconn *swconn;
* rconn_get_connection_seqno(rconn), 'swconn' has reconnected. */
static unsigned int conn_seq_no;
-static void pinctrl_handle_put_arp(const struct lport_index *,
- const struct flow *md,
+static void pinctrl_handle_put_arp(const struct flow *md,
const struct flow *headers);
-static void flush_put_arps(void);
-static void run_put_arps(struct controller_ctx *);
+static void init_put_arps(void);
+static void destroy_put_arps(void);
+static void run_put_arps(struct controller_ctx *,
+ const struct lport_index *lports);
static void wait_put_arps(struct controller_ctx *);
+static void flush_put_arps(void);
+
+COVERAGE_DEFINE(pinctrl_drop_put_arp);
void
pinctrl_init(void)
{
swconn = rconn_create(5, 0, DSCP_DEFAULT, 1 << OFP13_VERSION);
conn_seq_no = 0;
+ init_put_arps();
}
static ovs_be32
@@ -169,10 +175,6 @@ pinctrl_handle_arp(const struct flow *ip_flow, const struct match *md,
goto exit;
}
- struct ds s = DS_EMPTY_INITIALIZER;
- ofpacts_format(ofpacts.data, ofpacts.size, &s);
- ds_destroy(&s);
-
struct ofputil_packet_out po = {
.packet = dp_packet_data(&packet),
.packet_len = dp_packet_size(&packet),
@@ -190,8 +192,7 @@ exit:
}
static void
-process_packet_in(const struct lport_index *lports,
- const struct ofp_header *msg)
+process_packet_in(const struct ofp_header *msg)
{
static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
@@ -226,18 +227,18 @@ process_packet_in(const struct lport_index *lports,
break;
case ACTION_OPCODE_PUT_ARP:
- pinctrl_handle_put_arp(lports, &pin.flow_metadata.flow, &headers);
+ pinctrl_handle_put_arp(&pin.flow_metadata.flow, &headers);
break;
default:
- VLOG_WARN_RL(&rl, "unrecognized packet-in opcode %"PRIu32, ah->opcode);
+ VLOG_WARN_RL(&rl, "unrecognized packet-in opcode %"PRIu32,
+ ntohl(ah->opcode));
break;
}
}
static void
-pinctrl_recv(const struct lport_index *lports,
- const struct ofp_header *oh, enum ofptype type)
+pinctrl_recv(const struct ofp_header *oh, enum ofptype type)
{
if (type == OFPTYPE_ECHO_REQUEST) {
queue_msg(make_echo_reply(oh));
@@ -250,7 +251,7 @@ pinctrl_recv(const struct lport_index *lports,
config.miss_send_len = UINT16_MAX;
set_switch_config(swconn, &config);
} else if (type == OFPTYPE_PACKET_IN) {
- process_packet_in(lports, oh);
+ process_packet_in(oh);
} else if (type != OFPTYPE_ECHO_REPLY && type != OFPTYPE_BARRIER_REPLY) {
if (VLOG_IS_DBG_ENABLED()) {
static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(30, 300);
@@ -300,12 +301,12 @@ pinctrl_run(struct controller_ctx *ctx, const struct lport_index *lports,
enum ofptype type;
ofptype_decode(&type, oh);
- pinctrl_recv(lports, oh, type);
+ pinctrl_recv(oh, type);
ofpbuf_delete(msg);
}
}
- run_put_arps(ctx);
+ run_put_arps(ctx, lports);
}
void
@@ -320,7 +321,7 @@ void
pinctrl_destroy(void)
{
rconn_destroy(swconn);
- flush_put_arps();
+ destroy_put_arps();
}
/* Implementation of the "put_arp" OVN action. This action sends a packet to
@@ -333,103 +334,153 @@ pinctrl_destroy(void)
* we buffer up a few put_arps (but we don't keep them longer than 1 second)
* and apply them whenever a database transaction is available. */
-/* Buffered "put_arp" operations. */
+/* Buffered "put_arp" operation. */
struct put_arp {
+ struct hmap_node hmap_node; /* In 'put_arps'. */
+
long long int timestamp; /* In milliseconds. */
- char *logical_port;
+
+ /* Key. */
+ uint32_t dp_key;
+ uint32_t port_key;
ovs_be32 ip;
+
+ /* Value. */
struct eth_addr mac;
};
-static struct put_arp put_arps[1024];
-static size_t n_put_arps;
+
+/* Contains "struct put_arp"s. */
+static struct hmap put_arps;
static void
-pinctrl_handle_put_arp(const struct lport_index *lports,
- const struct flow *md, const struct flow *headers)
+init_put_arps(void)
{
- if (n_put_arps >= ARRAY_SIZE(put_arps)) {
- return;
+ hmap_init(&put_arps);
+}
+
+static void
+destroy_put_arps(void)
+{
+ flush_put_arps();
+ hmap_destroy(&put_arps);
+}
+
+static struct put_arp *
+pinctrl_find_put_arp(uint32_t dp_key, uint32_t port_key, ovs_be32 ip,
+ uint32_t hash)
+{
+ struct put_arp *pa;
+ HMAP_FOR_EACH_WITH_HASH (pa, hmap_node, hash, &put_arps) {
+ if (pa->dp_key == dp_key
+ && pa->port_key == port_key
+ && pa->ip == ip) {
+ return pa;
+ }
}
+ return NULL;
+}
- /* Convert logical datapath and logical port key into lport. */
+static void
+pinctrl_handle_put_arp(const struct flow *md, const struct flow *headers)
+{
uint32_t dp_key = ntohll(md->metadata);
uint32_t port_key = md->regs[MFF_LOG_INPORT - MFF_REG0];
- const struct sbrec_port_binding *pb
- = lport_lookup_by_key(lports, dp_key, port_key);
- if (!pb) {
- static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
+ ovs_be32 ip = htonl(md->regs[0]);
+ uint32_t hash = hash_3words(dp_key, port_key, (OVS_FORCE uint32_t) ip);
+ struct put_arp *pa = pinctrl_find_put_arp(dp_key, port_key, ip, hash);
+ if (!pa) {
+ if (hmap_count(&put_arps) >= 1000) {
+ COVERAGE_INC(pinctrl_drop_put_arp);
+ return;
+ }
- VLOG_WARN_RL(&rl, "unknown logical port with datapath %"PRIu32" and "
- "port %"PRIu32, dp_key, port_key);
- return;
+ pa = xmalloc(sizeof *pa);
+ hmap_insert(&put_arps, &pa->hmap_node, hash);
+ pa->dp_key = dp_key;
+ pa->port_key = port_key;
+ pa->ip = ip;
}
-
- struct put_arp *pa = &put_arps[n_put_arps++];
pa->timestamp = time_msec();
- pa->logical_port = xstrdup(pb->logical_port);
- pa->ip = htonl(md->regs[0]);
pa->mac = headers->dl_src;
}
static void
-flush_put_arps(void)
+run_put_arp(struct controller_ctx *ctx, const struct lport_index *lports,
+ const struct put_arp *pa)
{
- for (struct put_arp *pa = put_arps; pa < &put_arps[n_put_arps]; pa++) {
- free(pa->logical_port);
+ if (time_msec() > pa->timestamp + 1000) {
+ return;
}
- n_put_arps = 0;
-}
-static void
-run_put_arps(struct controller_ctx *ctx)
-{
- if (!ctx->ovnsb_idl_txn) {
+ /* Convert logical datapath and logical port key into lport. */
+ const struct sbrec_port_binding *pb
+ = lport_lookup_by_key(lports, pa->dp_key, pa->port_key);
+ if (!pb) {
+ static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
+
+ VLOG_WARN_RL(&rl, "unknown logical port with datapath %"PRIu32" "
+ "and port %"PRIu32, pa->dp_key, pa->port_key);
return;
}
- for (const struct put_arp *pa = put_arps; pa < &put_arps[n_put_arps];
- pa++) {
- if (time_msec() > pa->timestamp + 1000) {
- continue;
- }
+ /* Convert arguments to string form for database. */
+ char ip_string[INET_ADDRSTRLEN + 1];
+ snprintf(ip_string, sizeof ip_string, IP_FMT, IP_ARGS(pa->ip));
+
+ char mac_string[ETH_ADDR_STRLEN + 1];
+ snprintf(mac_string, sizeof mac_string,
+ ETH_ADDR_FMT, ETH_ADDR_ARGS(pa->mac));
- /* Convert arguments to string form for database. */
- char ip_string[INET_ADDRSTRLEN + 1];
- snprintf(ip_string, sizeof ip_string, IP_FMT, IP_ARGS(pa->ip));
- char mac_string[ETH_ADDR_STRLEN + 1];
- snprintf(mac_string, sizeof mac_string,
- ETH_ADDR_FMT, ETH_ADDR_ARGS(pa->mac));
-
- /* Check for and update an existing IP-MAC binding for this logical
- * port.
- *
- * XXX This is not very efficient. */
- const struct sbrec_mac_binding *b;
- SBREC_MAC_BINDING_FOR_EACH (b, ctx->ovnsb_idl) {
- if (!strcmp(b->logical_port, pa->logical_port)
- && !strcmp(b->ip, ip_string)) {
- if (strcmp(b->mac, mac_string)) {
- sbrec_mac_binding_set_mac(b, mac_string);
- }
- goto next;
+ /* Check for and update an existing IP-MAC binding for this logical
+ * port.
+ *
+ * XXX This is not very efficient. */
+ const struct sbrec_mac_binding *b;
+ SBREC_MAC_BINDING_FOR_EACH (b, ctx->ovnsb_idl) {
+ if (!strcmp(b->logical_port, pb->logical_port)
+ && !strcmp(b->ip, ip_string)) {
+ if (strcmp(b->mac, mac_string)) {
+ sbrec_mac_binding_set_mac(b, mac_string);
}
+ return;
}
+ }
+
+ /* Add new IP-MAC binding for this logical port. */
+ b = sbrec_mac_binding_insert(ctx->ovnsb_idl_txn);
+ sbrec_mac_binding_set_logical_port(b, pb->logical_port);
+ sbrec_mac_binding_set_ip(b, ip_string);
+ sbrec_mac_binding_set_mac(b, mac_string);
+}
- /* Add new IP-MAC binding for this logical port. */
- b = sbrec_mac_binding_insert(ctx->ovnsb_idl_txn);
- sbrec_mac_binding_set_logical_port(b, pa->logical_port);
- sbrec_mac_binding_set_ip(b, ip_string);
- sbrec_mac_binding_set_mac(b, mac_string);
- next:;
+static void
+run_put_arps(struct controller_ctx *ctx, const struct lport_index *lports)
+{
+ if (!ctx->ovnsb_idl_txn) {
+ return;
}
+ const struct put_arp *pa;
+ HMAP_FOR_EACH (pa, hmap_node, &put_arps) {
+ run_put_arp(ctx, lports, pa);
+ }
flush_put_arps();
}
static void
wait_put_arps(struct controller_ctx *ctx)
{
- if (ctx->ovnsb_idl_txn && n_put_arps) {
+ if (ctx->ovnsb_idl_txn && !hmap_is_empty(&put_arps)) {
poll_immediate_wake();
}
}
+
+static void
+flush_put_arps(void)
+{
+ struct put_arp *pa, *next;
+ HMAP_FOR_EACH_SAFE (pa, next, hmap_node, &put_arps) {
+ hmap_remove(&put_arps, &pa->hmap_node);
+ free(pa);
+ }
+}
@@ -685,7 +685,6 @@ icmp4 {
</p>
<pre>
-rate_limit(outport, ip4.dst);
arp {
eth.dst = ff:ff:ff:ff:ff:ff;
arp.spa = reg1;
@@ -698,6 +697,10 @@ arp {
(Ingress table 2 initialized <code>reg1</code> with the IP address
owned by <code>outport</code>.)
</p>
+
+ <p>
+ The IP packet that triggers the ARP request is dropped.
+ </p>
</li>
<li>
@@ -414,6 +414,11 @@ ovn_datapath_allocate_key(struct hmap *dp_tnlids)
return allocate_tnlid(dp_tnlids, "datapath", (1u << 24) - 1, &hint);
}
+/* Updates the southbound Datapath_Binding table so that it contains the
+ * logical switches and routers specified by the northbound database.
+ *
+ * Initializes 'datapaths' to contain a "struct ovn_datapath" for every logical
+ * switch and router. */
static void
build_datapaths(struct northd_context *ctx, struct hmap *datapaths)
{
@@ -701,6 +706,12 @@ ovn_port_update_sbrec(const struct ovn_port *op)
}
}
+/* Updates the southbound Port_Binding table so that it contains the logical
+ * ports specified by the northbound database.
+ *
+ * Initializes 'ports' to contain a "struct ovn_port" for every logical port,
+ * using the "struct ovn_datapath"s in 'datapaths' to look up logical
+ * datapaths. */
static void
build_ports(struct northd_context *ctx, struct hmap *datapaths,
struct hmap *ports)