Skip to content

Commit

Permalink
sol: require parameter req_channel_bw_mbps
Browse files Browse the repository at this point in the history 
The original configuration of SOL blocks assumes that
the bandwidth of the back interface of a Gatekeeper server
is representative of the bandwidth of the destination network.
This assumption does not hold in production deployments of
Gatekeeper.  For example, Gatekeeper servers typically operate
in networks with higher speeds (e.g., 40Gbps) than the protected
destination (e.g., 10Gbps).

This commit introduces the parameter destination_bw_gbps to address
this reality.
  • Loading branch information
@AltraMayor
AltraMayor committed May 21, 2024
1 parent 12244dd commit ce3864d
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 108 deletions.
7 changes: 0 additions & 7 deletions include/gatekeeper_sol.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,13 +102,6 @@ struct sol_config {
/* Maximum number of requests to store in priority queue at once. */
unsigned int pri_req_max_len;

/*
* Bandwidth limit for the priority queue of requests,
* as a percentage of the capacity of the link. Must
* be > 0 and < 1.
*/
double req_bw_rate;

/* Maximum request enqueue/dequeue size. */
unsigned int enq_burst_size;
unsigned int deq_burst_size;
Expand Down
1 change: 0 additions & 1 deletion lua/gatekeeper/staticlib.lua
View file
Original file line number Diff line number Diff line change
Expand Up @@ -414,7 +414,6 @@ struct dynamic_config {

struct sol_config {
unsigned int pri_req_max_len;
double req_bw_rate;
unsigned int enq_burst_size;
unsigned int deq_burst_size;
double tb_rate_approx_err;
Expand Down
5 changes: 2 additions & 3 deletions lua/sol.lua
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ return function (net_conf, sol_lcores)

-- These parameters should likely be initially changed.
local log_level = staticlib.c.RTE_LOG_DEBUG
local destination_bw_gbps = 1

-- XXX #155 These parameters should only be changed for performance reasons.
local log_ratelimit_interval_ms = 5000
Expand All @@ -17,7 +18,6 @@ return function (net_conf, sol_lcores)

-- These variables are unlikely to need to be changed.
local tb_rate_approx_err = 1e-7
local req_channel_bw_mbps = 0.0

--
-- End configuration of SOL block.
Expand All @@ -35,12 +35,11 @@ return function (net_conf, sol_lcores)
sol_conf.log_ratelimit_interval_ms = log_ratelimit_interval_ms
sol_conf.log_ratelimit_burst = log_ratelimit_burst
sol_conf.pri_req_max_len = pri_req_max_len
sol_conf.req_bw_rate = req_bw_rate
sol_conf.enq_burst_size = enq_burst_size
sol_conf.deq_burst_size = deq_burst_size

sol_conf.tb_rate_approx_err = tb_rate_approx_err
sol_conf.req_channel_bw_mbps = req_channel_bw_mbps
sol_conf.req_channel_bw_mbps = destination_bw_gbps * 1000 * req_bw_rate

local ret = staticlib.c.run_sol(net_conf, sol_conf)
if ret < 0 then
Expand Down
137 changes: 40 additions & 97 deletions sol/main.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -367,79 +367,23 @@ mbits_to_bytes(double mbps)
return mbps * (1000 * 1000 / 8);
}

/*
* Retrieve the link speed of a Gatekeeper interface. If it
* is a bonded interface, the link speeds are summed.
*/
static int
iface_speed_bytes(struct gatekeeper_if *iface, uint64_t *link_speed_bytes)
{
uint64_t link_speed_mbits = 0;
uint8_t i;
int ret;

RTE_VERIFY(link_speed_bytes != NULL);

for (i = 0; i < iface->num_ports; i++) {
struct rte_eth_link link;
ret = rte_eth_link_get(iface->ports[i], &link);
if (ret < 0) {
G_LOG(ERR, "net: querying port %hhu failed with err - %s\n",
iface->ports[i], rte_strerror(-ret));
goto err;
}

if (link.link_speed == RTE_ETH_SPEED_NUM_NONE ||
link.link_speed == RTE_ETH_SPEED_NUM_UNKNOWN) {
ret = -ENOTSUP;
goto err;
}

link_speed_mbits += link.link_speed;
}

/* Convert to bytes per second. */
*link_speed_bytes = mbits_to_bytes(link_speed_mbits);
return 0;

err:
*link_speed_bytes = 0;
return ret;
}

/*
* @sol_conf is allocated using rte_calloc_socket(), so initializations
* to 0 are not strictly necessary in this function.
*/
static int
req_queue_init(struct sol_config *sol_conf)
{
uint64_t link_speed_bytes;
double max_credit_bytes_precise;
double cycles_per_byte_precise;
uint64_t cycles_per_byte_floor;
uint64_t now;
uint32_t a, b;
int ret, i;

/* Find link speed in bytes, even for a bonded interface. */
ret = iface_speed_bytes(&sol_conf->net->back, &link_speed_bytes);
if (ret == 0) {
G_LOG(NOTICE,
"Back interface link speed: %"PRIu64" bytes per second\n",
link_speed_bytes);
/* Keep max number of bytes a float for later calculations. */
max_credit_bytes_precise =
sol_conf->req_bw_rate * link_speed_bytes;
} else {
G_LOG(NOTICE, "Back interface link speed: undefined\n");
if (sol_conf->req_channel_bw_mbps <= 0) {
G_LOG(ERR, "When link speed on back interface is undefined, parameter req_channel_bw_mbps must be calculated and defined\n");
return -1;
}
max_credit_bytes_precise =
mbits_to_bytes(sol_conf->req_channel_bw_mbps);
}
/* Convert to bytes per second. */
max_credit_bytes_precise =
mbits_to_bytes(sol_conf->req_channel_bw_mbps);

max_credit_bytes_precise /= sol_conf->num_lcores;

Expand All @@ -459,15 +403,16 @@ req_queue_init(struct sol_config *sol_conf)
cycles_per_byte_precise - cycles_per_byte_floor,
sol_conf->tb_rate_approx_err, &a, &b);
if (ret < 0) {
G_LOG(ERR, "Could not approximate the request queue's allocated bandwidth\n");
G_LOG(ERR, "%s(): could not approximate the request queue's allocated bandwidth\n",
__func__);
return ret;
}

/* Add integer number of cycles per byte to numerator. */
a += cycles_per_byte_floor * b;

G_LOG(NOTICE, "Cycles per byte (%f) represented as a rational: %u / %u\n",
cycles_per_byte_precise, a, b);
G_LOG(NOTICE, "%s(): cycles per byte (%f) represented as a rational: %u / %u\n",
__func__, cycles_per_byte_precise, a, b);

now = rte_rdtsc();

Expand Down Expand Up @@ -665,8 +610,10 @@ run_sol(struct net_config *net_conf, struct sol_config *sol_conf)
int ret, i;
uint16_t front_inc;

if (net_conf == NULL || sol_conf == NULL) {
ret = -1;
if (unlikely(net_conf == NULL || sol_conf == NULL)) {
G_LOG(ERR, "%s(): net_conf = %p or sol_conf = %p cannot be NULL\n",
__func__, net_conf, sol_conf);
ret = -EINVAL;
goto out;
}

Expand All @@ -677,51 +624,51 @@ run_sol(struct net_config *net_conf, struct sol_config *sol_conf)
sol_conf->log_level, "SOL");
}

if (!net_conf->back_iface_enabled) {
G_LOG(ERR, "Back interface is required\n");
ret = -1;
if (unlikely(!net_conf->back_iface_enabled)) {
G_LOG(ERR, "%s(): back interface is required\n", __func__);
ret = -EINVAL;
goto out;
}

if (sol_conf->pri_req_max_len == 0) {
G_LOG(ERR,
"Priority queue max len must be greater than 0\n");
ret = -1;
if (unlikely(sol_conf->pri_req_max_len == 0)) {
G_LOG(ERR, "%s(): the parameter pri_req_max_len = %u must be greater than 0\n",
__func__, sol_conf->pri_req_max_len);
ret = -EINVAL;
goto out;
}

if (sol_conf->enq_burst_size == 0 || sol_conf->deq_burst_size == 0) {
G_LOG(ERR, "Priority queue enqueue and dequeue sizes must both be greater than 0\n");
ret = -1;
if (unlikely(sol_conf->enq_burst_size == 0 ||
sol_conf->deq_burst_size == 0)) {
G_LOG(ERR, "%s(): the paramters enq_burst_size = %u and deq_burst_size = %u must both be greater than 0\n",
__func__, sol_conf->enq_burst_size,
sol_conf->deq_burst_size);
ret = -EINVAL;
goto out;
}

if (sol_conf->deq_burst_size > sol_conf->pri_req_max_len ||
sol_conf->enq_burst_size > sol_conf->pri_req_max_len) {
G_LOG(ERR, "Request queue enqueue and dequeue sizes must be less than the max length of the request queue\n");
ret = -1;
if (unlikely(sol_conf->enq_burst_size > sol_conf->pri_req_max_len ||
sol_conf->deq_burst_size > sol_conf->pri_req_max_len)) {
G_LOG(ERR, "%s(): the paramters enq_burst_size = %u and deq_burst_size = %u must both be less than or equal to the parameter pri_req_max_len = %u\n",
__func__, sol_conf->enq_burst_size,
sol_conf->deq_burst_size, sol_conf->pri_req_max_len);
ret = -EINVAL;
goto out;
}

if (sol_conf->req_bw_rate <= 0 || sol_conf->req_bw_rate >= 1) {
G_LOG(ERR,
"Request queue bandwidth must be in range (0, 1), but it has been specified as %f\n",
sol_conf->req_bw_rate);
ret = -1;
if (unlikely(sol_conf->req_channel_bw_mbps <= 0)) {
G_LOG(ERR, "%s(): the parameter req_channel_bw_mbps = %f must be greater than 0\n",
__func__, sol_conf->req_channel_bw_mbps);
ret = -EINVAL;
goto out;
}

if (sol_conf->req_channel_bw_mbps < 0) {
G_LOG(ERR,
"Request channel bandwidth in Mbps must be greater than 0 when the NIC doesn't supply guaranteed bandwidth, but is %f\n",
sol_conf->req_channel_bw_mbps);
ret = -1;
if (unlikely(sol_conf->num_lcores <= 0)) {
G_LOG(ERR, "%s(): the parameter num_lcores = %i must be greater than 0\n",
__func__, sol_conf->num_lcores);
ret = -EINVAL;
goto out;
}

if (sol_conf->num_lcores <= 0)
goto success;

/*
* Need to account for the packets in the following scenarios:
*
Expand Down Expand Up @@ -758,8 +705,8 @@ run_sol(struct net_config *net_conf, struct sol_config *sol_conf)
}

sol_conf->net = net_conf;

goto success;
rte_atomic32_init(&sol_conf->ref_cnt);
return 0;

stage2:
pop_n_at_stage2(1);
Expand All @@ -769,10 +716,6 @@ run_sol(struct net_config *net_conf, struct sol_config *sol_conf)
net_conf->front.total_pkt_burst -= front_inc;
out:
return ret;

success:
rte_atomic32_init(&sol_conf->ref_cnt);
return 0;
}

/*
Expand Down

0 comments on commit ce3864d

Please sign in to comment.