Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug fix: Flow Directory. (test_flow_dir) #248

Open
wants to merge 33 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 31 commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
dfbc44f
Bug fix: Not setting action after adding to table.
Jul 14, 2020
66d1725
Bug fix: Not setting action after adding to table.
Jul 14, 2020
f6d04a9
Removed memset when filling key in flow dir
Jul 17, 2020
6bf658d
Merge bug fix
Jul 17, 2020
524db7f
Styling
Jul 17, 2020
289eaf3
Update onvm ipv4 tuple size.
Jul 17, 2020
785a682
Convert port to big endian when filling key.
Jul 19, 2020
0888f1a
Initialize flow entry to NULL & new feature
Jul 20, 2020
ac08651
Conform to Linter
Jul 20, 2020
179afc0
Update print default service chain.
Jul 20, 2020
a312d3f
Update flow entry variable to be global.
Jul 20, 2020
d88f412
Update destination structs
Jul 21, 2020
1c20d9e
Update readme with new functionality.
bdevierno1 Jul 21, 2020
6e419cb
Removed memset
Jul 22, 2020
7cc7a61
Merge
Jul 22, 2020
94446c6
Conform to Linter
Jul 22, 2020
31447bd
Linter: whitespace
Jul 22, 2020
ed8681c
Merge dev
Jul 23, 2020
887a277
Merge dev
Jul 23, 2020
7c7ae8f
First review. TODO: Resolve memset placement.
Jul 23, 2020
05dff7b
Revert pkt helper changes
Jul 23, 2020
785ee44
Tested with service chains.
Jul 24, 2020
92092ee
Test flow dir.c Ready for review.
Jul 28, 2020
7c79874
Styling and undo changes to speed tester.
Jul 28, 2020
90b77a1
Set Next action when destination is 255
Jul 28, 2020
98b4530
Use pkt rss
Jul 28, 2020
c92b77a
Documentation update. Service ID 255.
bdevierno1 Jul 28, 2020
d113696
Define 255
Jul 29, 2020
0ff3053
Style
Jul 29, 2020
02cf1a4
Update to use latest pktgen
Jul 30, 2020
faa11e6
Merge upstream/develop
Aug 3, 2020
61bff85
Merge remote-tracking branch 'upstream/develop' into test_flow_dir
Jan 14, 2021
965329a
Remove multiple defn for var flow_entry
Jan 14, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions examples/test_flow_dir/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ Flow Director Test

This NF demonstrates how to use ONVM's Flow Director. When a packet arrives the NF checks whether it is from a flow that already has a service chain rule. If not, it creates a new rule so the packet will be sent to the destination NF indicated on the command line. Packets that match a rule are processed with the ONVM_NF_ACTION_NEXT action.

This NF demonstrates how to populate the Manager's flow table as well as how to create service chain rules for specific flows. To enable this functionality use the -s flag.

Compilation and Execution
--
```
Expand All @@ -24,6 +26,7 @@ App Specific Arguments
--
- `-d <dst>`: destination service ID to foward to
- `-p <print_delay>`: number of packets between each print, e.g. `-p 1` prints every packets.
- `s`: Prepopulate sample flow table rules.

Config File Support
--
Expand Down
105 changes: 98 additions & 7 deletions examples/test_flow_dir/test_flow_dir.c
Original file line number Diff line number Diff line change
Expand Up @@ -66,11 +66,18 @@

#define NF_TAG "test_flow_dir"

/* number of package between each print */
/* Number of packets between each print. */
static uint32_t print_delay = 1000000;

static uint32_t destination;

/* Populate flow disabled by default */
static int populate_flow = 0;

static struct onvm_flow_entry *flow_entry = NULL;

/* Pointer to the manager flow table */
extern struct onvm_ft* sdn_ft;
/*
* Print a usage message
*/
Expand All @@ -82,6 +89,8 @@ usage(const char *progname) {
printf("Flags:\n");
printf(" - `-d <dst>`: destination service ID to foward to\n");
printf(" - `-p <print_delay>`: number of packets between each print, e.g. `-p 1` prints every packets.\n");
printf(" - -s : Prepopulate sample flow table rules. \n");

}

/*
Expand All @@ -91,14 +100,17 @@ static int
parse_app_args(int argc, char *argv[], const char *progname) {
int c;

while ((c = getopt(argc, argv, "d:p:")) != -1) {
while ((c = getopt(argc, argv, "d:p:s")) != -1) {
switch (c) {
case 'd':
destination = strtoul(optarg, NULL, 10);
break;
case 'p':
print_delay = strtoul(optarg, NULL, 10);
break;
case 's':
populate_flow = 1;
break;
case '?':
usage(progname);
if (optopt == 'd')
Expand Down Expand Up @@ -151,32 +163,103 @@ do_stats_display(struct rte_mbuf *pkt) {
}
}

/*
* This function populates a set of flows to the sdn flow table.
* For each new flow a service chain is created and appended.
* Each service chain is of max 4 length. This function is not enabled by default.
* Users may update the predefined struct with their own rules here.
*/

static void
populate_sample_ipv4(void) {
struct onvm_flow_entry *flow_entry = (struct onvm_flow_entry *)
rte_calloc(NULL, 1, sizeof(struct onvm_flow_entry), 0);
if (flow_entry == NULL) {
rte_exit(EXIT_FAILURE, "Unable to populate flow.\n");
}
struct test_add_key {
struct onvm_ft_ipv4_5tuple key;
uint8_t destinations[ONVM_MAX_CHAIN_LENGTH];
};

struct test_add_key keys[] = {
{{RTE_IPV4(100, 10, 0, 0), RTE_IPV4(100, 10, 0, 0), 1, 1, IPPROTO_TCP}, {2,3}},
{{RTE_IPV4(102, 10, 0, 0), RTE_IPV4(101, 10, 0, 1), 0, 1, IPPROTO_TCP}, {4,3,2,1}},
{{RTE_IPV4(103, 10, 0, 0), RTE_IPV4(102, 10, 0, 1), 0, 1, IPPROTO_TCP}, {2,1}},
{{RTE_IPV4(10, 11, 1, 17), RTE_IPV4(10, 11, 1, 17), 1234, 1234, IPPROTO_UDP}, {4,3,2}},
};

uint32_t num_keys = RTE_DIM(keys);
for (uint32_t i = 0; i < num_keys; i++) {
struct onvm_ft_ipv4_5tuple key;
key = keys[i].key;
bdevierno1 marked this conversation as resolved.
Show resolved Hide resolved
int ret = onvm_ft_lookup_key(sdn_ft, &key, (char **)&flow_entry);
if (ret >= 0) {
continue;
} else {
printf("\nAdding Key: ");
_onvm_ft_print_key(&key);
ret = onvm_ft_add_key(sdn_ft, &key, (char **)&flow_entry);
if (ret < 0) {
printf("Unable to add key.");
continue;
}
printf("Creating new service chain.\n");
flow_entry->sc = onvm_sc_create();
uint32_t num_dest = RTE_DIM(keys[i].destinations);
for (uint32_t j = 0; j < num_dest; j++) {
if (keys[i].destinations[j] == 0) {
continue;
}
printf("Appending Destination: %d \n", keys[i].destinations[j]);
onvm_sc_append_entry(flow_entry->sc, ONVM_NF_ACTION_TONF, keys[i].destinations[j]);
}
}
}
flow_entry = NULL;
rte_free(flow_entry);
bdevierno1 marked this conversation as resolved.
Show resolved Hide resolved
}

static void
nf_setup(__attribute__((unused)) struct onvm_nf_local_ctx *nf_local_ctx) {
flow_entry = (struct onvm_flow_entry *) rte_calloc(NULL, 1, sizeof(struct onvm_flow_entry), 0);
if (flow_entry == NULL) {
rte_exit(EXIT_FAILURE, "Unable to allocate flow entry\n");
}
}

static int
packet_handler(struct rte_mbuf *pkt, struct onvm_pkt_meta *meta,
__attribute__((unused)) struct onvm_nf_local_ctx *nf_local_ctx) {
static uint32_t counter = 0;
struct onvm_flow_entry *flow_entry = NULL;
int ret;

if (!onvm_pkt_is_ipv4(pkt)) {
meta->action = ONVM_NF_ACTION_DROP;
return 0;
}

if (++counter == print_delay) {
do_stats_display(pkt);
counter = 0;
}

ret = onvm_flow_dir_get_pkt(pkt, &flow_entry);
struct onvm_ft_ipv4_5tuple key;
onvm_ft_fill_key(&key, pkt);
bdevierno1 marked this conversation as resolved.
Show resolved Hide resolved
ret = onvm_ft_lookup_key(sdn_ft, &key, (char **)&flow_entry);
if (ret >= 0) {
meta->action = ONVM_NF_ACTION_NEXT;
} else {
ret = onvm_flow_dir_add_pkt(pkt, &flow_entry);
ret = onvm_ft_add_key(sdn_ft, &key, (char **)&flow_entry);
if (ret < 0) {
meta->action = ONVM_NF_ACTION_DROP;
meta->destination = 0;
return 0;
}
memset(flow_entry, 0, sizeof(struct onvm_flow_entry));
flow_entry->sc = onvm_sc_create();
onvm_sc_append_entry(flow_entry->sc, ONVM_NF_ACTION_TONF, destination);
// onvm_sc_print(flow_entry->sc);
meta->action = ONVM_NF_ACTION_TONF;
meta->destination = destination;
bdevierno1 marked this conversation as resolved.
Show resolved Hide resolved
}
return 0;
}
Expand All @@ -193,6 +276,7 @@ main(int argc, char *argv[]) {

nf_function_table = onvm_nflib_init_nf_function_table();
nf_function_table->pkt_handler = &packet_handler;
nf_function_table->setup = &nf_setup;

if ((arg_offset = onvm_nflib_init(argc, argv, NF_TAG, nf_local_ctx, nf_function_table)) < 0) {
onvm_nflib_stop(nf_local_ctx);
Expand All @@ -214,9 +298,16 @@ main(int argc, char *argv[]) {
/* Map the sdn_ft table */
onvm_flow_dir_nf_init();

if (populate_flow) {
printf("Populating flow table. \n");
populate_sample_ipv4();
}

onvm_nflib_run(nf_local_ctx);

onvm_nflib_stop(nf_local_ctx);
flow_entry = NULL;
rte_free(flow_entry);
bdevierno1 marked this conversation as resolved.
Show resolved Hide resolved
printf("If we reach here, program is ending\n");
return 0;
}
2 changes: 1 addition & 1 deletion onvm/onvm_mgr/onvm_init.c
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ init(int argc, char *argv[]) {
printf("Chain length can not be larger than the maximum chain length\n");
exit(1);
}
printf("Default service chain: send to sdn NF\n");
printf("Default service chain: send to NF with service ID 1\n");

/* set up service chain pointer shared to NFs*/
mz_scp = rte_memzone_reserve(MZ_SCP_INFO, sizeof(struct onvm_service_chain *), rte_socket_id(), NO_FLAGS);
Expand Down
2 changes: 2 additions & 0 deletions onvm/onvm_nflib/onvm_common.h
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,8 @@
#define ONVM_NF_ACTION_TONF 2 // send to the NF specified in the argument field (assume it is on the same host)
#define ONVM_NF_ACTION_OUT 3 // send the packet out the NIC port set in the argument field

#define ACTION_NEXT_DEST_ID 255 // Destination service ID that will set the manager to perform flow table lookup.

#define PKT_WAKEUP_THRESHOLD 1 // for shared core mode, how many packets are required to wake up the NF
#define MSG_WAKEUP_THRESHOLD 1 // for shared core mode, how many messages on an NF's ring are required to wake up the NF

Expand Down
28 changes: 14 additions & 14 deletions onvm/onvm_nflib/onvm_flow_table.h
Original file line number Diff line number Diff line change
Expand Up @@ -126,10 +126,10 @@ onvm_ft_free(struct onvm_ft *table);

static inline void
_onvm_ft_print_key(struct onvm_ft_ipv4_5tuple *key) {
printf("IP: %" PRIu8 ".%" PRIu8 ".%" PRIu8 ".%" PRIu8, key->src_addr & 0xFF, (key->src_addr >> 8) & 0xFF,
(key->src_addr >> 16) & 0xFF, (key->src_addr >> 24) & 0xFF);
printf("-%" PRIu8 ".%" PRIu8 ".%" PRIu8 ".%" PRIu8 " ", key->dst_addr & 0xFF, (key->dst_addr >> 8) & 0xFF,
(key->dst_addr >> 16) & 0xFF, (key->dst_addr >> 24) & 0xFF);
printf("IP: %" PRIu8 ".%" PRIu8 ".%" PRIu8 ".%" PRIu8, (key->src_addr >> 24) & 0xFF,
(key->src_addr >> 16) & 0xFF, (key->src_addr >> 8) & 0xFF, key->src_addr & 0xFF);
printf("-%" PRIu8 ".%" PRIu8 ".%" PRIu8 ".%" PRIu8 " ", (key->dst_addr >> 24) & 0xFF,
(key->dst_addr >> 16) & 0xFF, (key->dst_addr >> 8) & 0xFF, key->dst_addr & 0xFF);
printf("Port: %d %d Proto: %d\n", key->src_port, key->dst_port, key->proto);
}

Expand All @@ -150,16 +150,16 @@ onvm_ft_fill_key(struct onvm_ft_ipv4_5tuple *key, struct rte_mbuf *pkt) {
ipv4_hdr = onvm_pkt_ipv4_hdr(pkt);
memset(key, 0, sizeof(struct onvm_ft_ipv4_5tuple));
bdevierno1 marked this conversation as resolved.
Show resolved Hide resolved
key->proto = ipv4_hdr->next_proto_id;
key->src_addr = ipv4_hdr->src_addr;
key->dst_addr = ipv4_hdr->dst_addr;
key->src_addr = rte_be_to_cpu_32(ipv4_hdr->src_addr);
key->dst_addr = rte_be_to_cpu_32(ipv4_hdr->dst_addr);
if (key->proto == IP_PROTOCOL_TCP) {
tcp_hdr = onvm_pkt_tcp_hdr(pkt);
key->src_port = tcp_hdr->src_port;
key->dst_port = tcp_hdr->dst_port;
key->src_port = rte_be_to_cpu_16(tcp_hdr->src_port);
key->dst_port = rte_be_to_cpu_16(tcp_hdr->dst_port);
} else if (key->proto == IP_PROTOCOL_UDP) {
udp_hdr = onvm_pkt_udp_hdr(pkt);
key->src_port = udp_hdr->src_port;
key->dst_port = udp_hdr->dst_port;
key->src_port = rte_be_to_cpu_16(udp_hdr->src_port);
key->dst_port = rte_be_to_cpu_16(udp_hdr->dst_port);
} else {
key->src_port = 0;
key->dst_port = 0;
Expand Down Expand Up @@ -224,10 +224,10 @@ onvm_softrss(struct onvm_ft_ipv4_5tuple *key) {

rte_convert_rss_key((uint32_t *)rss_symmetric_key, (uint32_t *)rss_key_be, RTE_DIM(rss_symmetric_key));

tuple.v4.src_addr = rte_be_to_cpu_32(key->src_addr);
tuple.v4.dst_addr = rte_be_to_cpu_32(key->dst_addr);
tuple.v4.sport = rte_be_to_cpu_16(key->src_port);
tuple.v4.dport = rte_be_to_cpu_16(key->dst_port);
tuple.v4.src_addr = key->src_addr;
tuple.v4.dst_addr = key->dst_addr;
tuple.v4.sport = key->src_port;
tuple.v4.dport = key->dst_port;

rss_l3l4 = rte_softrss_be((uint32_t *)&tuple, RTE_THASH_V4_L4_LEN, rss_key_be);

Expand Down
3 changes: 3 additions & 0 deletions onvm/onvm_nflib/onvm_nflib.c
Original file line number Diff line number Diff line change
Expand Up @@ -555,6 +555,9 @@ onvm_nflib_start_nf(struct onvm_nf_local_ctx *nf_local_ctx, struct onvm_nf_init_

int
onvm_nflib_run(struct onvm_nf_local_ctx *nf_local_ctx) {
/* Map the sdn_ft table */
onvm_flow_dir_nf_init();

int ret;

pthread_t main_loop_thread;
Expand Down
5 changes: 2 additions & 3 deletions onvm/onvm_nflib/onvm_pkt_common.c
Original file line number Diff line number Diff line change
Expand Up @@ -102,9 +102,8 @@ onvm_pkt_process_tx_batch(struct queue_mgr *tx_mgr, struct rte_mbuf *pkts[], uin
// and !<return value> is 1.
nf->stats.act_drop++;
nf->stats.tx += !onvm_pkt_drop(pkts[i]);
} else if (meta->action == ONVM_NF_ACTION_NEXT) {
/* TODO: Here we drop the packet : there will be a flow table
in the future to know what to do with the packet next */
} else if (meta->action == ONVM_NF_ACTION_NEXT || meta->destination == ACTION_NEXT_DEST_ID) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bdevierno1 setting 255 as the next action would definitely decrease performance, but I think that (from CI) we see that just having to process this or || statement in the condition slows down other NFs. Because we know that speed_tester would not have set the meta->destination to 255. I think that's the cause of the ~4% performance drop from CI

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah this is correct. Unfortunately, I tried different ways to optimize this but did not see any substantial improvements. By removing the OR I will still have to set another conditional which will have to be placed before this statement so as to maintain the same logic. Perhaps there is another way to do this?

// Perform next action is configured by the manager's flow table
nf->stats.act_next++;
onvm_pkt_process_next_action(tx_mgr, pkts[i], nf);
} else if (meta->action == ONVM_NF_ACTION_TONF) {
Expand Down