Lotsa revision, not done yet.

- Updated many comments.
- Noticed several errors, added more TODOs (identifiable by the
"issue #65" label).
- Deleted some functions no longer being used.
Also some pointless whims:
- Changed the name of the BIB lock because it was too wordy.

include/nat64/mod/bib_db.h

@@ -7,13 +7,13 @@
  * Formally defined in RFC 6146 section 3.1.
  *
  * @author Alberto Leiva
+ * @author Daniel Hernandez
  */
 
-#include <linux/spinlock.h>
 #include "nat64/comm/types.h"
 #include "nat64/mod/packet.h"
 
-/******************************** bib.h *************************************/
+/******************************** BIB Entries *************************************/
 
 
 /**
@@ -44,48 +44,55 @@ struct bib_entry {
 	bool is_static;
 
 	/**
-	 * Number of active references to this entry, excluding the BIB database's. When this reaches
-	 * zero, the entry is removed from the database and freed.
+	 * Number of active references to this entry, excluding the ones from the table it belongs to.
+	 * When this reaches zero, the entry is removed from the table and freed.
 	 */
 	struct kref refcounter;
 
+	/** Appends this entry to the database's IPv6 index. */
 	struct rb_node tree6_hook;
+	/** Appends this entry to the database's IPv4 index. */
 	struct rb_node tree4_hook;
 };
 
 /**
- * Initializes the kmem_cache for efficient allocation.
- * Call during initialization for the remaining functions to work properly.
- */
-int bib_init(void);
-
-/**
- * Empties the kmem_cache.
- * Call during destruction to avoid memory leaks.
- */
-void bib_destroy(void);
-
-/**
- * Helper function, intended to initialize a BIB entry.
- * The entry is generated IN DYNAMIC MEMORY (if you end up not inserting it to a BIB table, you need
- * to bib_kfree() it).
+ * Allocates and initializes a BIB entry.
+ * The entry is generated in dynamic memory; remember to kfree, return or pass it along.
  */
 struct bib_entry *bib_create(struct ipv4_tuple_address *ipv4, struct ipv6_tuple_address *ipv6,
 		bool is_static, l4_protocol l4_proto);
+/**
+ * Roughly reverts the work of bib_create() by freeing "bib" from memory. What breaks the symmetry
+ * is the return of "bib"'s IPv4 address to the IPv4 pool (the borrow doesn't happen in
+ * bib_create()).
+ *
+ * This is intended to be used when you are the only user of "bib" (i.e. you just created it
+ * and you haven't inserted it to any tables). If that might not be the case, use bib_return()
+ * instead.
+ */
+void bib_kfree(struct bib_entry *bib);
 
 /**
- * Helper function, intended to increment a BIB refcounter
+ * Marks "bib" as being used by the caller. The idea is to prevent the cleaners from deleting it
+ * while it's being used.
+ *
+ * You have to grab one of these references whenever you gain access to an entry. Keep in mind that
+ * the bib* and bibdb* functions might have already done that for you. Session entries referencing
+ * BIB entries must also count.
+ *
+ * Remove the mark when you're done by calling bib_return().
  */
 void bib_get(struct bib_entry *bib);
 /**
- * Helper function, intended to decrement a BIB refcounter
+ * Reverts the work of bib_get() by removing the mark.
+ *
+ * If no other references to "bib" exist, this function will take care of removing and freeing it.
+ *
+ * DON'T USE "bib" AFTER YOU RETURN IT!
  */
 int bib_return(struct bib_entry *bib);
+int bib_return_lockless(struct bib_entry *bib);
 
-/**
- * Warning: Careful with this one; "bib" cannot be NULL.
- */
-void bib_kfree(struct bib_entry *bib);
 
 /**
  * Make sure you use bib_get or bibdb_get before you use
@@ -95,7 +102,7 @@ void bib_kfree(struct bib_entry *bib);
 int bib_session_counter(struct bib_entry *bib);
 
 
-/************************* End of bib.h *************************************/
+/************************* BIB (The database) *************************************/
 
 /**
  * Initializes the three tables (UDP, TCP and ICMP).
@@ -116,7 +123,7 @@ void bibdb_destroy(void);
  * When we're translating from IPv4 to IPv6, "result" will point to the BIB whose IPv4 address is
  * "tuple"'s destination address.
  *
- * It increases "result"'s refcount. Make sure you release it when you're done.
+ * It increases "result"'s refcount. Make sure you decrement it when you're done.
  *
  * @param[in] tuple summary of the packet. Describes the BIB you need.
  * @param[out] the BIB entry you'd expect from the "tuple" tuple.
@@ -128,7 +135,7 @@ int bibdb_get(struct tuple *tuple, struct bib_entry **result);
  * Makes "result" point to the BIB entry from the "l4_proto" table whose IPv4 side (address and
  * port) is "addr".
  *
- * It increases "result"'s refcount. Make sure you release it when you're done.
+ * It increases "result"'s refcount. Make sure you decrement it when you're done.
  *
  * @param[in] address address and port you want the BIB entry for.
  * @param[in] l4_proto identifier of the table to retrieve the entry from.
@@ -162,11 +169,9 @@ int bibdb_get_by_ipv6(struct ipv6_tuple_address *addr, l4_protocol l4_proto,
 int bibdb_get_or_create_ipv6(struct fragment *frag, struct tuple *tuple, struct bib_entry **bib);
 
 /**
- * Adds "in_bib" to the BIB table whose layer-4 protocol is "l4_proto".
+ * Adds "entry" to the BIB table whose layer-4 protocol is "l4_proto".
  * Expects all fields from "entry" to have been initialized.
  *
- * Because never in this project is required otherwise, assumes the entry is not yet on the table.
- *
  * The table's references are not supposed to count towards the entries' refcounts. Do free your
  * reference if your entry made it into the table; do not assume you're transferring it.
  *
@@ -178,14 +183,13 @@ int bibdb_get_or_create_ipv6(struct fragment *frag, struct tuple *tuple, struct
 int bibdb_add(struct bib_entry *entry, l4_protocol l4_proto);
 
 /**
- * Attempts to remove the "entry" entry from the BIB table whose protocol is "l4_proto".
- * Even though the entry is removed from the table, it is not kfreed.
+ * Attempts to remove the "entry" entry from its BIB. It doesn't kfree "entry".
  *
  * @param entry row to be removed from the table.
- * @param l4_proto identifier of the table to remove "entry" from.
+ * @param lock TODO (issue #65)
  * @return error status.
  */
-int bibdb_remove(struct bib_entry *entry, l4_protocol l4_proto);
+int bibdb_remove(struct bib_entry *entry, bool lock);
 
 /**
  * Runs the "func" function for every entry in the table whose protocol is "l4_proto".
@@ -206,6 +210,5 @@ int bibdb_count(l4_protocol proto, __u64 *result);
 int bibdb_get_or_create_ipv6(struct fragment *frag, struct tuple *tuple, struct bib_entry **bib);
 int bibdb_delete_by_ipv4(struct in_addr *addr);
 
-int biddb_exists_on_addr(struct in_addr *addr);
 
 #endif /* _NF_NAT64_BIB_DB_H */

include/nat64/mod/rbtree.h

@@ -108,10 +108,8 @@
 	}
 
 /**
- * This is just a stock search on a Red-Black tree.
- *
- * I can't find a way to turn this into a function; if you want to read a cleaner version of it,
- * see https://www.kernel.org/doc/Documentation/rbtree.txt.
+ * Similar to rbtree_find(), except if it doesn't find the node it returns the slot where it'd be
+ * placed so you can insert something in there.
  */
 #define rbtree_find_node(expected, root, compare_cb, type, hook_name, parent, node) \
 	({ \

include/nat64/mod/session_db.h

@@ -7,12 +7,13 @@
  * Formally defined in RFC 6146 section 3.2.
  *
  * @author Alberto Leiva
+ * @author Daniel Hernandez
  */
 
 #include "nat64/comm/types.h"
 #include "nat64/mod/bib_db.h"
 
-/************************************* session.h **********************************/
+/************************************* Session Entries **********************************/
 
 /** The states from the TCP state machine; RFC 6146 section 3.5.2. */
 enum tcp_states {
@@ -58,10 +59,16 @@ struct session_entry {
 	/**
 	 * Owner bib of this session. Used for quick access during removal.
 	 * (when the session dies, the BIB might have to die too.)
+	 *
+	 * TODO (issue #65) this comment might not be entirely up-to-date.
 	 */
 	struct bib_entry *bib;
 
-	/** A reference counter related to this session. */
+	/**
+	 * A reference counter related to this session.
+	 *
+	 * TODO (issue #65) expand this comment.
+	 */
 	struct kref refcounter;
 	/**
 	 * Chainer to one of the expiration timer lists (sessions_udp, sessions_tcp_est, etc).
@@ -74,49 +81,52 @@ struct session_entry {
 	 */
 	l4_protocol l4_proto;
 
-	/** Current TCP state.
-	 * 	Each STE represents a state machine
-	 */
+	/** Current TCP state. Only relevant if l4_proto == L4PROTO_TCP. */
 	u_int8_t state;
 
+	/** Appends this entry to the database's IPv6 index. */
 	struct rb_node tree6_hook;
+	/** Appends this entry to the database's IPv4 index. */
 	struct rb_node tree4_hook;
 };
 
 /**
- * Initializes the three tables (UDP, TCP and ICMP).
- * Call during initialization for the remaining functions to work properly.
- */
-int session_init(void);
-/**
- * Empties the session tables, freeing any memory being used by them.
- * Call during destruction to avoid memory leaks.
- */
-void session_destroy(void);
-
-/**
- * Helper function, intended to increment a BIB refcounter
+ * Marks "session" as being used by the caller. The idea is to prevent the cleaners from deleting
+ * it while it's being used.
+ *
+ * You have to grab one of these references whenever you gain access to an entry. Keep in mind that
+ * the session* and sessiondb* functions might have already done that for you.
+ *
+ * Remove the mark when you're done by calling session_return().
  */
 void session_get(struct session_entry *session);
 /**
- * Helper function, intended to decrement a BIB refcounter
+ * Reverts the work of session_get() by removing the mark.
+ *
+ * If no other references to "session" exist, this function will take care of removing and freeing
+ * it.
+ *
+ * DON'T USE "session" AFTER YOU RETURN IT!
  */
 int session_return(struct session_entry *session);
 
 /**
- * Helper function, intended to initialize a Session entry.
- * The entry is generated IN DYNAMIC MEMORY (if you end up not inserting it to a Session table, you
- * need to session_kfree() it).
+ * Allocates and initializes a session entry.
+ * The entry is generated in dynamic memory; remember to kfree, return or pass it along.
  */
 struct session_entry *session_create(struct ipv4_pair *ipv4, struct ipv6_pair *ipv6,
 		l4_protocol l4_proto);
 /**
- * Warning: Careful with this one; "session" cannot be NULL.
+ * Reverts the work of session_create() by freeing "session" from memory.
+ *
+ * This is intended to be used when you are the only user of "session" (i.e. you just created it
+ * and you haven't inserted it to any tables). If that might not be the case, use session_return()
+ * instead.
  */
 void session_kfree(struct session_entry *session);
 
 
-/********************************* End of session.h *******************************/
+/********************************* Session Database *******************************/
 
 typedef enum timer_type {
 	TIMERTYPE_UDP = 0,
@@ -128,12 +138,10 @@ typedef enum timer_type {
 } timer_type;
 
 /**
- * Initializes the three tables (UDP, TCP and ICMP).
  * Call during initialization for the remaining functions to work properly.
  */
 int sessiondb_init(void);
 /**
- * Empties the session tables, freeing any memory being used by them.
  * Call during destruction to avoid memory leaks.
  */
 void sessiondb_destroy(void);
@@ -143,7 +151,9 @@ void sessiondb_destroy(void);
  * Returns in "result" the session entry from the "l4_proto" table whose IPv4 side (both addresses
  * and ports) is "pair".
  *
- * @param[in] pairt IPv4 data you want the session entry for.
+ * It increases "result"'s refcount. Make sure you decrement it when you're done.
+ *
+ * @param[in] pair IPv4 data you want the session entry for.
  * @param[in] l4_proto identifier of the table to retrieve the entry from.
  * @param[out] result the Session entry from the "l4_proto" table whose IPv4 side (both addresses
  *		and ports) is "address".
@@ -155,6 +165,8 @@ int sessiondb_get_by_ipv4(struct ipv4_pair *pair, l4_protocol l4_proto,
  * Returns in "result" the session entry from the "l4_proto" table whose IPv6 side (both addresses
  * and ports) is "pair".
  *
+ * It increases "result"'s refcount. Make sure you decrement it when you're done.
+ *
  * @param[in] pairt IPv6 data you want the session entry for.
  * @param[in] l4_proto identifier of the table to retrieve the entry from.
  * @param[out] result the Session entry from the "l4_proto" table whose IPv6 side (both addresses
@@ -164,9 +176,10 @@ int sessiondb_get_by_ipv4(struct ipv4_pair *pair, l4_protocol l4_proto,
 int sessiondb_get_by_ipv6(struct ipv6_pair *pair, l4_protocol l4_proto,
 		struct session_entry **result);
 /**
- * Returns in "result" the session entry you'd expect from the "tuple" tuple.
+ * Returns in "result" the session entry you'd expect from the "tuple" tuple. That is, looks ups
+ * the session entry by both source and destination addresses.
  *
- * That is, looks ups the session entry by both source and destination addresses.
+ * It increases "result"'s refcount. Make sure you release it when you're done.
  *
  * @param[in] tuple summary of the packet. Describes the session you need.
  * @param[out] result the session entry you'd expect from the "tuple" tuple.
@@ -192,15 +205,11 @@ int sessiondb_get(struct tuple *tuple, struct session_entry **result);
 bool sessiondb_allow(struct tuple *tuple);
 
 /**
- * Adds "in_session" to the session table whose layer-4 protocol is "entry->l4_proto".
- * Expects all fields but the list_heads from "entry" to have been initialized.
+ * Adds "session" to the database. Expects all fields but the list_heads from "entry" to have been
+ * initialized.
  *
- * if the in_session is added to the table, "tree_session" will point to "in_session",
- * otherwise "tree_session" will point to a session of the table.
- *
- * @param entry row to be added to the table.
- * @return whether the entry could be inserted or not. It will not be inserted
- *		if some dynamic memory allocation failed.
+ * @param session row to be added to the table.
+ * @return error status.
  */
 int sessiondb_add(struct session_entry *session);