Add more comments to explain the ALG API (#35)

Author: rmind

src/kern/npf_alg.c

@@ -1,5 +1,5 @@
 /*-
- * Copyright (c) 2010-2013 The NetBSD Foundation, Inc.
+ * Copyright (c) 2010-2019 The NetBSD Foundation, Inc.
  * All rights reserved.
  *
  * This material is based upon work partially supported by The
@@ -40,8 +40,6 @@ __KERNEL_RCSID(0, "$NetBSD: npf_alg.c,v 1.19 2019/01/19 21:19:31 rmind Exp $");
 
 #include <sys/kmem.h>
 #include <sys/pserialize.h>
-#include <sys/mutex.h>
-#include <net/pfil.h>
 #include <sys/module.h>
 #endif
 
@@ -55,20 +53,20 @@ __KERNEL_RCSID(0, "$NetBSD: npf_alg.c,v 1.19 2019/01/19 21:19:31 rmind Exp $");
 
 struct npf_alg {
 	const char *	na_name;
-	u_int		na_slot;
+	unsigned	na_slot;
 };
 
 struct npf_algset {
 	/* List of ALGs and the count. */
 	npf_alg_t	alg_list[NPF_MAX_ALGS];
-	u_int		alg_count;
+	unsigned	alg_count;
 
 	/* Matching, inspection and translation functions. */
 	npfa_funcs_t	alg_funcs[NPF_MAX_ALGS];
 };
 
-static const char	alg_prefix[] = "npf_alg_";
-#define	NPF_EXT_PREFLEN	(sizeof(alg_prefix) - 1)
+#define	NPF_ALG_PREF	"npf_alg_"
+#define	NPF_ALG_PREFLEN	(sizeof(NPF_ALG_PREF) - 1)
 
 void
 npf_alg_init(npf_t *npf)
@@ -94,7 +92,7 @@ npf_alg_lookup(npf_t *npf, const char *name)
 
 	KASSERT(npf_config_locked_p(npf));
 
-	for (u_int i = 0; i < aset->alg_count; i++) {
+	for (unsigned i = 0; i < aset->alg_count; i++) {
 		npf_alg_t *alg = &aset->alg_list[i];
 		const char *aname = alg->na_name;
 
@@ -111,9 +109,9 @@ npf_alg_construct(npf_t *npf, const char *name)
 
 	npf_config_enter(npf);
 	if ((alg = npf_alg_lookup(npf, name)) == NULL) {
-		char modname[NPF_EXT_PREFLEN + 64];
+		char modname[NPF_ALG_PREFLEN + 64];
 
-		snprintf(modname, sizeof(modname), "%s%s", alg_prefix, name);
+		snprintf(modname, sizeof(modname), "%s%s", NPF_ALG_PREF, name);
 		npf_config_exit(npf);
 
 		if (module_autoload(modname, MODULE_CLASS_MISC) != 0) {
@@ -135,7 +133,7 @@ npf_alg_register(npf_t *npf, const char *name, const npfa_funcs_t *funcs)
 	npf_algset_t *aset = npf->algset;
 	npfa_funcs_t *afuncs;
 	npf_alg_t *alg;
-	u_int i;
+	unsigned i;
 
 	npf_config_enter(npf);
 	if (npf_alg_lookup(npf, name) != NULL) {
@@ -178,7 +176,7 @@ int
 npf_alg_unregister(npf_t *npf, npf_alg_t *alg)
 {
 	npf_algset_t *aset = npf->algset;
-	u_int i = alg->na_slot;
+	unsigned i = alg->na_slot;
 	npfa_funcs_t *afuncs;
 
 	/* Deactivate the functions first. */
@@ -198,7 +196,16 @@ npf_alg_unregister(npf_t *npf, npf_alg_t *alg)
 }
 
 /*
- * npf_alg_match: call ALG matching inspectors, determine if any ALG matches.
+ * npf_alg_match: call the ALG matching inspectors.
+ *
+ *	The purpose of the "matching" inspector function in the ALG API
+ *	is to determine whether this connection matches the ALG criteria
+ *	i.e. is concerning the ALG.  If yes, ALG can associate itself with
+ *	the given NAT state structure and set/save an arbitrary parameter.
+ *	This is done using the using the npf_nat_setalg() function.
+ *
+ *	=> This is called when the packet matches the dynamic NAT policy
+ *	   and the NAT state entry is being created for it [NAT-ESTABLISH].
  */
 bool
 npf_alg_match(npf_cache_t *npc, npf_nat_t *nt, int di)
@@ -207,8 +214,10 @@ npf_alg_match(npf_cache_t *npc, npf_nat_t *nt, int di)
 	bool match = false;
 	int s;
 
+	KASSERTMSG(npf_iscached(npc, NPC_IP46), "expecting protocol number");
+
 	s = pserialize_read_enter();
-	for (u_int i = 0; i < aset->alg_count; i++) {
+	for (unsigned i = 0; i < aset->alg_count; i++) {
 		const npfa_funcs_t *f = &aset->alg_funcs[i];
 
 		if (f->match && f->match(npc, nt, di)) {
@@ -221,16 +230,26 @@ npf_alg_match(npf_cache_t *npc, npf_nat_t *nt, int di)
 }
 
 /*
- * npf_alg_exec: execute ALG hooks for translation.
+ * npf_alg_exec: execute the ALG translation processors.
+ *
+ *	The ALG function would perform any additional packet translation
+ *	or manipulation here.  The translate function will be called by
+ *	once the ALG has been associated with the NAT state through the
+ *	npf_alg_match() inspector.
+ *
+ *	=> This is called when the packet is being translated according
+ *	   to the dynamic NAT logic [NAT-TRANSLATE].
  */
 void
 npf_alg_exec(npf_cache_t *npc, npf_nat_t *nt, bool forw)
 {
 	npf_algset_t *aset = npc->npc_ctx->algset;
 	int s;
 
+	KASSERTMSG(npf_iscached(npc, NPC_IP46), "expecting protocol number");
+
 	s = pserialize_read_enter();
-	for (u_int i = 0; i < aset->alg_count; i++) {
+	for (unsigned i = 0; i < aset->alg_count; i++) {
 		const npfa_funcs_t *f = &aset->alg_funcs[i];
 
 		if (f->translate) {
@@ -240,6 +259,23 @@ npf_alg_exec(npf_cache_t *npc, npf_nat_t *nt, bool forw)
 	pserialize_read_exit(s);
 }
 
+/*
+ * npf_alg_conn: query ALGs giving which may perform a custom state lookup.
+ *
+ *	The purpose of ALG connection inspection function is to provide
+ *	ALGs with a mechanism to override the regular connection state
+ *	lookup, if they need to.  For example, some ALGs may want to
+ *	extract and use a different 5-tuple to perform a lookup.
+ *
+ *	=> This is called at the beginning of the connection state lookup
+ *	   function [CONN-LOOKUP].
+ *
+ *	=> Must use the npf_conn_lookup() function to perform the custom
+ *	   connection state lookup and return the result.
+ *
+ *	=> Returning NULL will result in NPF performing a regular state
+ *	   lookup for the packet.
+ */
 npf_conn_t *
 npf_alg_conn(npf_cache_t *npc, int di)
 {
@@ -248,7 +284,7 @@ npf_alg_conn(npf_cache_t *npc, int di)
 	int s;
 
 	s = pserialize_read_enter();
-	for (u_int i = 0; i < aset->alg_count; i++) {
+	for (unsigned i = 0; i < aset->alg_count; i++) {
 		const npfa_funcs_t *f = &aset->alg_funcs[i];
 
 		if (!f->inspect)
@@ -260,14 +296,17 @@ npf_alg_conn(npf_cache_t *npc, int di)
 	return con;
 }
 
+/*
+ * npf_alg_export: serialise the configuration of ALGs.
+ */
 int
 npf_alg_export(npf_t *npf, nvlist_t *npf_dict)
 {
 	npf_algset_t *aset = npf->algset;
 
 	KASSERT(npf_config_locked_p(npf));
 
-	for (u_int i = 0; i < aset->alg_count; i++) {
+	for (unsigned i = 0; i < aset->alg_count; i++) {
 		const npf_alg_t *alg = &aset->alg_list[i];
 		nvlist_t *algdict;
 

src/kern/npf_connkey.c

@@ -159,7 +159,7 @@ npf_conn_conkey(const npf_cache_t *npc, npf_connkey_t *key, const bool forw)
 	const unsigned alen = npc->npc_alen;
 	const struct tcphdr *th;
 	const struct udphdr *uh;
-	uint16_t id[2];
+	uint16_t id[2] = { 0, 0 };
 
 	switch (proto) {
 	case IPPROTO_TCP: