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 27 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
2 changes: 2 additions & 0 deletions docs/NF_Dev.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ Each NF needs to have a packet handler function. It must match this specificati
- `ONVM_NF_ACTION_NEXT`: Forward the packet using the rule from the SDN controller stored in the flow table
- `ONVM_NF_ACTION_TONF`: Forward the packet to the specified NF
- `ONVM_NF_ACTION_OUT`: Forward the packet to the specified NIC port

NFs that set destination ID to 255 will be forwarded with the `ONVM_NF_ACTION_NEXT` action.

NF Library
--
Expand Down
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[] = {
{{IPv4(100, 10, 0, 0), IPv4(100, 10, 0, 0), 1, 1, IPPROTO_TCP}, {2,3}},
{{IPv4(102, 10, 0, 0), IPv4(101, 10, 0, 1), 0, 1, IPPROTO_TCP}, {4,3,2,1}},
{{IPv4(103, 10, 0, 0), IPv4(102, 10, 0, 1), 0, 1, IPPROTO_TCP}, {2,1}},
{{IPv4(10, 11, 1, 17), 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
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
2 changes: 1 addition & 1 deletion onvm/onvm_nflib/onvm_pkt_common.c
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ 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) {
} else if (meta->action == ONVM_NF_ACTION_NEXT || meta->destination == 255) {
Copy link
Member

Choose a reason for hiding this comment

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

No magic numbers please, what is this 255? Why do we need it?
Can't the packet just set action ONVM_NF_ACTION_NEXT?

Copy link
Member

Choose a reason for hiding this comment

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

Having a special number is purposeful since we want to use this to indicate the next action even if the NF wasn't designed for that purpose (eg this lets us make the SimpleFwd NF use the "next action" behavior if you set 255 at the command line as the destination ID).

But you are right that it shouldn't be totally magic -- @bdevierno1 you should #define a constant somewhere for this instead of using 255 directly here.

Copy link
Member

@koolzz koolzz Jul 28, 2020

Choose a reason for hiding this comment

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

Sure, feels like a lazy way to change the action(we could just bake in the same if meta->destination ==255 -> meta->action = ONVM_NF_ACTION_NEXT into the NF itself) but if people find it easier lets define a global const.

Copy link
Member

Choose a reason for hiding this comment

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

@bdevierno1 Also if we're doing this, after #defining the constant (probably in common.h) do a onvm mgr startup check to confirm that your const number is more than MAX_NFS. Otherwise if someone has 512 NFS configured they will be suprised that forwarding to 256 doesn't work

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This line was resulting in the perf drop with speed tester. I couldn't find another way to optimize this besides moving the ONVM_NF_ACTION_TONF if statement above the one for the NEXT action on line 105. But I think this would feel like it would be an over-optimization for speed tester? I'll update to use the global const for now. I could also update all the sample NFs to use a flag instead to indicate to use the next action as well if we decide that's better.

/* TODO: Here we drop the packet : there will be a flow table
in the future to know what to do with the packet next */
nf->stats.act_next++;
Expand Down