From owner-svn-src-all@freebsd.org Thu Nov 23 08:02:03 2017 Return-Path: Delivered-To: svn-src-all@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id C3DF2DE34D9; Thu, 23 Nov 2017 08:02:03 +0000 (UTC) (envelope-from ae@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 80F1C19E8; Thu, 23 Nov 2017 08:02:03 +0000 (UTC) (envelope-from ae@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id vAN822UC076932; Thu, 23 Nov 2017 08:02:02 GMT (envelope-from ae@FreeBSD.org) Received: (from ae@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id vAN822St076928; Thu, 23 Nov 2017 08:02:02 GMT (envelope-from ae@FreeBSD.org) Message-Id: <201711230802.vAN822St076928@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: ae set sender to ae@FreeBSD.org using -f From: "Andrey V. Elsukov" Date: Thu, 23 Nov 2017 08:02:02 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r326118 - head/sys/netpfil/ipfw X-SVN-Group: head X-SVN-Commit-Author: ae X-SVN-Commit-Paths: head/sys/netpfil/ipfw X-SVN-Commit-Revision: 326118 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.25 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 23 Nov 2017 08:02:03 -0000 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);