AltraMayor / AltraMayor/gatekeeper

Generalized IP addresses

Open
#629 0 comments 0 reactions 0 assignees View on GitHub
enhancement
Dominant language
C
Stars
1.6k
Forks
252
PR merge metrics
No merged PRs in 30d

Description

Thanks to pull requests #594 and #628, Gatekeeper has RIB and FIB libraries that require little code adjustments to handle IPv4 and IPv6. The importance of higher levels of code reuse is direct: less code to write and maintain, and lower effort to verify the correctness and to test. The gains brought by the new RIB and FIB libraries can be extended if Gatekeeper adopts a generalized way to handle IP addresses.

Gatekeeper currently relies on `struct in_addr` and `struct in_addr` to store IPv4 and IPv6, respectively. This approach requires different ways to use and access the addresses. For example, consider the functions `lookup_fib_bulk()` and `lookup_fib6_bulk()`, both in `gk/main.c`. Although flows are passed to these functions using the generalized `struct ip_flow`, accessing the destination addresses is not identical: `(const uint8_t *)&flows[i]->f.v4.dst.s_addr` vs `flows[i]->f.v6.dst.s6_addr`. For reference, here is the definition of `struct ip_flow`:
```c
struct ip_flow {
/* IPv4 or IPv6. */
uint16_t proto;

union {
struct {
struct in_addr src;
struct in_addr dst;
} v4;

struct {
struct in6_addr src;
struct in6_addr dst;
} v6;
} f;
};
```
A solution to this problem is to adopt `rib_address_t` to store all IP addresses. With this new approach, `struct ip_flow` would become as follows:
```c
struct ip_flow {
/* IPv4 or IPv6. */
uint16_t proto;
/* Addresses in network order. */
rib_address_t src_no;
rib_address_t dst_no;
};
```
Of course, a number of support functions are necessary to handle these addresses without distinction. And some of these support functions are already available in the code of the RIB and FIB libraries. For example: `read_addr()`, `write_addr()`, and `address_to_str()`.

Once IP addresses are generalized, one can refactor the codebase to leverage code reuse. For example, `struct gk_lpm`, which encapsulates the routing table for IPv4 and IPv6 in the GK block, could be broken up into two instances of a new `struct gk_lpm`. The new `struct gk_lpm` would be as follows:
```c
struct gk_lpm {
/* Use a spin lock to edit the FIB table. */
rte_spinlock_t lock;

/* The RIB and FIB. */
struct fib_head fib;

/* The FIB table that decides the actions on packets. */
struct gk_fib *fib_tbl;

/*
* Indexes of the last FIB entries allocated at @fib_tbl and @fib_tbl6.
* get_empty_fib_id() is the only function that uses these fields.
*/
uint32_t last_index;

/* The number of entries in field @fib_tbl. */
uint32_t num_fib;
};
```
Notice that, since there will be two instances of the new `struct gk_lpm`, there will a lock for IPv4 and a lock for IPv6. In addition, the field `num_fib` was added to remove dependence on fields `max_num_ipv4_rules` and `max_num_ipv6_rules` of `struct gk_config`.

The new `struct gk_lpm` would enable `lookup_fib_bulk()` and `lookup_fib6_bulk()` to become a single function. Code everywhere in the codebase has potential for deduplication using a similar strategy; especially code in `gk/rt.c` and code that relies on `struct ipaddr`.

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.