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

Generalized IP addresses #629

Open
AltraMayor opened this issue Feb 3, 2023 · 0 comments
Open

Generalized IP addresses #629

AltraMayor opened this issue Feb 3, 2023 · 0 comments

Comments

@AltraMayor
Copy link
Owner

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:

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:

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:

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.

@AltraMayor AltraMayor added this to the Code refactoring milestone Feb 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant