svn commit: r326118 - head/sys/netpfil/ipfw

Andrey V. Elsukov ae at FreeBSD.org
Thu Nov 23 08:02:03 UTC 2017


Author: ae
Date: Thu Nov 23 08:02:02 2017
New Revision: 326118
URL: https://svnweb.freebsd.org/changeset/base/326118

Log:
  Modify ipfw's dynamic states KPI.
  
  Hide the locking logic used in the dynamic states implementation from
  generic code. Rename ipfw_install_state() and ipfw_lookup_dyn_rule()
  function to have similar names: ipfw_dyn_install_state() and
  ipfw_dyn_lookup_state(). Move dynamic rule counters updating to the
  ipfw_dyn_lookup_state() function. Now this function return NULL when
  there is no state and pointer to the parent rule when state is found.
  Thus now there is no need to return pointer to dynamic rule, and no need
  to hold bucket lock for this state. Remove ipfw_dyn_unlock() function.
  
  Obtained from:	Yandex LLC
  MFC after:	1 week
  Sponsored by:	Yandex LLC
  Differential Revision:	https://reviews.freebsd.org/D11657

Modified:
  head/sys/netpfil/ipfw/ip_fw2.c
  head/sys/netpfil/ipfw/ip_fw_dynamic.c
  head/sys/netpfil/ipfw/ip_fw_private.h

Modified: head/sys/netpfil/ipfw/ip_fw2.c
==============================================================================
--- head/sys/netpfil/ipfw/ip_fw2.c	Thu Nov 23 07:05:25 2017	(r326117)
+++ head/sys/netpfil/ipfw/ip_fw2.c	Thu Nov 23 08:02:02 2017	(r326118)
@@ -1122,7 +1122,7 @@ ipfw_chk(struct ip_fw_args *args)
 	 */
 	int dyn_dir = MATCH_UNKNOWN;
 	uint16_t dyn_name = 0;
-	ipfw_dyn_rule *q = NULL;
+	struct ip_fw *q = NULL;
 	struct ip_fw_chain *chain = &V_layer3_chain;
 
 	/*
@@ -2307,7 +2307,7 @@ do {								\
 			 */
 			case O_LIMIT:
 			case O_KEEP_STATE:
-				if (ipfw_install_state(chain, f,
+				if (ipfw_dyn_install_state(chain, f,
 				    (ipfw_insn_limit *)cmd, args, tablearg)) {
 					/* error or limit violation */
 					retval = IP_FW_DENY;
@@ -2347,28 +2347,25 @@ do {								\
 				if ((dyn_dir == MATCH_UNKNOWN ||
 				    (dyn_name != 0 &&
 				    dyn_name != cmd->arg1)) &&
-				    (q = ipfw_lookup_dyn_rule(&args->f_id,
-				     &dyn_dir, proto == IPPROTO_TCP ?
-				     TCP(ulp): NULL,
+				    (q = ipfw_dyn_lookup_state(&args->f_id,
+				     ulp, pktlen, &dyn_dir,
 				     (dyn_name = cmd->arg1))) != NULL) {
 					/*
-					 * Found dynamic entry, update stats
-					 * and jump to the 'action' part of
-					 * the parent rule by setting
-					 * f, cmd, l and clearing cmdlen.
+					 * Found dynamic entry, jump to the
+					 * 'action' part of the parent rule
+					 * by setting f, cmd, l and clearing
+					 * cmdlen.
 					 */
-					IPFW_INC_DYN_COUNTER(q, pktlen);
+					f = q;
 					/* XXX we would like to have f_pos
 					 * readily accessible in the dynamic
 				         * rule, instead of having to
 					 * lookup q->rule.
 					 */
-					f = q->rule;
 					f_pos = ipfw_find_rule(chain,
-						f->rulenum, f->id);
+					    f->rulenum, f->id);
 					cmd = ACTION_PTR(f);
 					l = f->cmd_len - f->act_ofs;
-					ipfw_dyn_unlock(q);
 					cmdlen = 0;
 					match = 1;
 					break;
@@ -2580,8 +2577,7 @@ do {								\
 			case O_FORWARD_IP:
 				if (args->eh)	/* not valid on layer2 pkts */
 					break;
-				if (q == NULL || q->rule != f ||
-				    dyn_dir == MATCH_FORWARD) {
+				if (q != f || dyn_dir == MATCH_FORWARD) {
 				    struct sockaddr_in *sa;
 
 				    sa = &(((ipfw_insn_sa *)cmd)->sa);
@@ -2641,8 +2637,7 @@ do {								\
 			case O_FORWARD_IP6:
 				if (args->eh)	/* not valid on layer2 pkts */
 					break;
-				if (q == NULL || q->rule != f ||
-				    dyn_dir == MATCH_FORWARD) {
+				if (q != f || dyn_dir == MATCH_FORWARD) {
 					struct sockaddr_in6 *sin6;
 
 					sin6 = &(((ipfw_insn_sa6 *)cmd)->sa);

Modified: head/sys/netpfil/ipfw/ip_fw_dynamic.c
==============================================================================
--- head/sys/netpfil/ipfw/ip_fw_dynamic.c	Thu Nov 23 07:05:25 2017	(r326117)
+++ head/sys/netpfil/ipfw/ip_fw_dynamic.c	Thu Nov 23 08:02:02 2017	(r326118)
@@ -251,7 +251,7 @@ SYSEND
 
 #ifdef INET6
 static __inline int
-hash_packet6(struct ipfw_flow_id *id)
+hash_packet6(const struct ipfw_flow_id *id)
 {
 	u_int32_t i;
 	i = (id->dst_ip6.__u6_addr.__u6_addr32[2]) ^
@@ -268,7 +268,7 @@ hash_packet6(struct ipfw_flow_id *id)
  * and we want to find both in the same bucket.
  */
 static __inline int
-hash_packet(struct ipfw_flow_id *id, int buckets)
+hash_packet(const struct ipfw_flow_id *id, int buckets)
 {
 	u_int32_t i;
 
@@ -476,8 +476,8 @@ static struct opcode_obj_rewrite dyn_opcodes[] = {
  * Print customizable flow id description via log(9) facility.
  */
 static void
-print_dyn_rule_flags(struct ipfw_flow_id *id, int dyn_type, int log_flags,
-    char *prefix, char *postfix)
+print_dyn_rule_flags(const struct ipfw_flow_id *id, int dyn_type,
+    int log_flags, char *prefix, char *postfix)
 {
 	struct in_addr da;
 #ifdef INET6
@@ -511,12 +511,14 @@ print_dyn_rule_flags(struct ipfw_flow_id *id, int dyn_
 
 static void
 dyn_update_proto_state(ipfw_dyn_rule *q, const struct ipfw_flow_id *id,
-    const struct tcphdr *tcp, int dir)
+    const void *ulp, int dir)
 {
+	const struct tcphdr *tcp;
 	uint32_t ack;
 	u_char flags;
 
 	if (id->proto == IPPROTO_TCP) {
+		tcp = (const struct tcphdr *)ulp;
 		flags = id->_flags & (TH_FIN | TH_SYN | TH_RST);
 #define BOTH_SYN	(TH_SYN | (TH_SYN << 8))
 #define BOTH_FIN	(TH_FIN | (TH_FIN << 8))
@@ -592,8 +594,8 @@ dyn_update_proto_state(ipfw_dyn_rule *q, const struct 
  * Lookup a dynamic rule, locked version.
  */
 static ipfw_dyn_rule *
-lookup_dyn_rule_locked(struct ipfw_flow_id *pkt, int i, int *match_direction,
-    struct tcphdr *tcp, uint16_t kidx)
+lookup_dyn_rule_locked(const struct ipfw_flow_id *pkt, const void *ulp,
+    int i, int *match_direction, uint16_t kidx)
 {
 	/*
 	 * Stateful ipfw extensions.
@@ -660,41 +662,35 @@ lookup_dyn_rule_locked(struct ipfw_flow_id *pkt, int i
 	}
 
 	/* update state according to flags */
-	dyn_update_proto_state(q, pkt, tcp, dir);
+	dyn_update_proto_state(q, pkt, ulp, dir);
 done:
 	if (match_direction != NULL)
 		*match_direction = dir;
 	return (q);
 }
 
-ipfw_dyn_rule *
-ipfw_lookup_dyn_rule(struct ipfw_flow_id *pkt, int *match_direction,
-    struct tcphdr *tcp, uint16_t kidx)
+struct ip_fw *
+ipfw_dyn_lookup_state(const struct ipfw_flow_id *pkt, const void *ulp,
+    int pktlen, int *match_direction, uint16_t kidx)
 {
+	struct ip_fw *rule;
 	ipfw_dyn_rule *q;
 	int i;
 
 	i = hash_packet(pkt, V_curr_dyn_buckets);
 
 	IPFW_BUCK_LOCK(i);
-	q = lookup_dyn_rule_locked(pkt, i, match_direction, tcp, kidx);
+	q = lookup_dyn_rule_locked(pkt, ulp, i, match_direction, kidx);
 	if (q == NULL)
-		IPFW_BUCK_UNLOCK(i);
-	/* NB: return table locked when q is not NULL */
-	return q;
+		rule = NULL;
+	else {
+		rule = q->rule;
+		IPFW_INC_DYN_COUNTER(q, pktlen);
+	}
+	IPFW_BUCK_UNLOCK(i);
+	return (rule);
 }
 
-/*
- * Unlock bucket mtx
- * @p - pointer to dynamic rule
- */
-void
-ipfw_dyn_unlock(ipfw_dyn_rule *q)
-{
-
-	IPFW_BUCK_UNLOCK(q->bucket);
-}
-
 static int
 resize_dynamic_table(struct ip_fw_chain *chain, int nbuckets)
 {
@@ -787,7 +783,7 @@ resize_dynamic_table(struct ip_fw_chain *chain, int nb
  * - "parent" rules for the above (O_LIMIT_PARENT).
  */
 static ipfw_dyn_rule *
-add_dyn_rule(struct ipfw_flow_id *id, int i, uint8_t dyn_type,
+add_dyn_rule(const struct ipfw_flow_id *id, int i, uint8_t dyn_type,
     struct ip_fw *rule, uint16_t kidx)
 {
 	ipfw_dyn_rule *r;
@@ -837,8 +833,8 @@ add_dyn_rule(struct ipfw_flow_id *id, int i, uint8_t d
  * If the lookup fails, then install one.
  */
 static ipfw_dyn_rule *
-lookup_dyn_parent(struct ipfw_flow_id *pkt, int *pindex, struct ip_fw *rule,
-    uint16_t kidx)
+lookup_dyn_parent(const struct ipfw_flow_id *pkt, int *pindex,
+    struct ip_fw *rule, uint16_t kidx)
 {
 	ipfw_dyn_rule *q;
 	int i, is_v6;
@@ -882,7 +878,7 @@ lookup_dyn_parent(struct ipfw_flow_id *pkt, int *pinde
  * session limitations are enforced.
  */
 int
-ipfw_install_state(struct ip_fw_chain *chain, struct ip_fw *rule,
+ipfw_dyn_install_state(struct ip_fw_chain *chain, struct ip_fw *rule,
     ipfw_insn_limit *cmd, struct ip_fw_args *args, uint32_t tablearg)
 {
 	ipfw_dyn_rule *q;
@@ -895,7 +891,7 @@ ipfw_install_state(struct ip_fw_chain *chain, struct i
 
 	IPFW_BUCK_LOCK(i);
 
-	q = lookup_dyn_rule_locked(&args->f_id, i, NULL, NULL, cmd->o.arg1);
+	q = lookup_dyn_rule_locked(&args->f_id, NULL, i, NULL, cmd->o.arg1);
 	if (q != NULL) {	/* should never occur */
 		DEB(
 		if (last_log != time_uptime) {

Modified: head/sys/netpfil/ipfw/ip_fw_private.h
==============================================================================
--- head/sys/netpfil/ipfw/ip_fw_private.h	Thu Nov 23 07:05:25 2017	(r326117)
+++ head/sys/netpfil/ipfw/ip_fw_private.h	Thu Nov 23 08:02:02 2017	(r326118)
@@ -183,15 +183,14 @@ struct ip_fw_chain;
 struct sockopt_data;
 int ipfw_is_dyn_rule(struct ip_fw *rule);
 void ipfw_expire_dyn_rules(struct ip_fw_chain *, ipfw_range_tlv *);
-void ipfw_dyn_unlock(ipfw_dyn_rule *q);
 
 struct tcphdr;
 struct mbuf *ipfw_send_pkt(struct mbuf *, struct ipfw_flow_id *,
     u_int32_t, u_int32_t, int);
-int ipfw_install_state(struct ip_fw_chain *chain, struct ip_fw *rule,
+int ipfw_dyn_install_state(struct ip_fw_chain *chain, struct ip_fw *rule,
     ipfw_insn_limit *cmd, struct ip_fw_args *args, uint32_t tablearg);
-ipfw_dyn_rule *ipfw_lookup_dyn_rule(struct ipfw_flow_id *pkt,
-	int *match_direction, struct tcphdr *tcp, uint16_t kidx);
+struct ip_fw *ipfw_dyn_lookup_state(const struct ipfw_flow_id *pkt,
+    const void *ulp, int pktlen, int *match_direction, uint16_t kidx);
 void ipfw_remove_dyn_children(struct ip_fw *rule);
 void ipfw_get_dynamic(struct ip_fw_chain *chain, char **bp, const char *ep);
 int ipfw_dump_states(struct ip_fw_chain *chain, struct sockopt_data *sd);


More information about the svn-src-all mailing list