From owner-svn-src-projects@FreeBSD.ORG Tue Sep 2 14:27:13 2014 Return-Path: Delivered-To: svn-src-projects@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 99718813; Tue, 2 Sep 2014 14:27:13 +0000 (UTC) Received: from svn.freebsd.org (svn.freebsd.org [IPv6:2001:1900:2254:2068::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 797D41DF5; Tue, 2 Sep 2014 14:27:13 +0000 (UTC) Received: from svn.freebsd.org ([127.0.1.70]) by svn.freebsd.org (8.14.9/8.14.9) with ESMTP id s82ERD3C063138; Tue, 2 Sep 2014 14:27:13 GMT (envelope-from melifaro@FreeBSD.org) Received: (from melifaro@localhost) by svn.freebsd.org (8.14.9/8.14.9/Submit) id s82ERCY5063135; Tue, 2 Sep 2014 14:27:12 GMT (envelope-from melifaro@FreeBSD.org) Message-Id: <201409021427.s82ERCY5063135@svn.freebsd.org> X-Authentication-Warning: svn.freebsd.org: melifaro set sender to melifaro@FreeBSD.org using -f From: "Alexander V. Chernikov" Date: Tue, 2 Sep 2014 14:27:12 +0000 (UTC) To: src-committers@freebsd.org, svn-src-projects@freebsd.org Subject: svn commit: r270968 - projects/ipfw/sys/netpfil/ipfw X-SVN-Group: projects MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-projects@freebsd.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: "SVN commit messages for the src " projects" tree" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 02 Sep 2014 14:27:13 -0000 Author: melifaro Date: Tue Sep 2 14:27:12 2014 New Revision: 270968 URL: http://svnweb.freebsd.org/changeset/base/270968 Log: Add more comments on newly-added functions. Add back opstate handler function. Modified: projects/ipfw/sys/netpfil/ipfw/ip_fw_table.c projects/ipfw/sys/netpfil/ipfw/ip_fw_table.h projects/ipfw/sys/netpfil/ipfw/ip_fw_table_value.c Modified: projects/ipfw/sys/netpfil/ipfw/ip_fw_table.c ============================================================================== --- projects/ipfw/sys/netpfil/ipfw/ip_fw_table.c Tue Sep 2 14:26:25 2014 (r270967) +++ projects/ipfw/sys/netpfil/ipfw/ip_fw_table.c Tue Sep 2 14:27:12 2014 (r270968) @@ -481,8 +481,59 @@ flush_batch_buffer(struct ip_fw_chain *c free(ta_buf_m, M_TEMP); } + +static void +rollback_add_entry(void *object, struct op_state *_state) +{ + struct ip_fw_chain *ch; + struct tableop_state *ts; + + ts = (struct tableop_state *)_state; + + if (ts->tc != object && ts->ch != object) + return; + + ch = ts->ch; + + IPFW_UH_WLOCK_ASSERT(ch); + + /* Call specifid unlockers */ + rollback_table_values(ts); + + /* Indicate we've called */ + ts->modified = 1; +} + /* * Adds/updates one or more entries in table @ti. + * + * Function may drop/reacquire UH wlock multiple times due to + * items alloc, algorithm callbacks (check_space), value linkage + * (new values, value storage realloc), etc.. + * Other processes like other adds (which may involve storage resize), + * table swaps (which changes table data and may change algo type), + * table modify (which may change value mask) may be executed + * simultaneously so we need to deal with it. + * + * The following approach was implemented: + * we have per-chain linked list, protected with UH lock. + * add_table_entry prepares special on-stack structure wthich is passed + * to its descendants. Users add this structure to this list before unlock. + * After performing needed operations and acquiring UH lock back, each user + * checks if structure has changed. If true, it rolls local state back and + * returns without error to the caller. + * add_table_entry() on its own checks if structure has changed and restarts + * its operation from the beginning (goto restart). + * + * Functions which are modifying fields of interest (currently + * resize_shared_value_storage() and swap_tables() ) + * traverses given list while holding UH lock immediately before + * performing their operations calling function provided be list entry + * ( currently rollback_add_entry ) which performs rollback for all necessary + * state and sets appropriate values in structure indicating rollback + * has happened. + * + * Algo interaction: * Function references @ti first to ensure table won't * disappear or change its type. * After that, prepare_add callback is called for each @tei entry. @@ -526,6 +577,7 @@ restart: } ta = tc->ta; ts.ch = ch; + ts.opstate.func = rollback_add_entry; ts.tc = tc; ts.vshared = tc->vshared; ts.vmask = tc->vmask; @@ -624,7 +676,7 @@ restart: IPFW_WUNLOCK(ch); - ipfw_finalize_table_values(ch, tc, tei, count, rollback); + ipfw_garbage_table_values(ch, tc, tei, count, rollback); /* Permit post-add algorithm grow/rehash. */ if (numadd != 0) @@ -714,7 +766,7 @@ del_table_entry(struct ip_fw_chain *ch, IPFW_WUNLOCK(ch); /* Unlink non-used values */ - ipfw_finalize_table_values(ch, tc, tei, count, 0); + ipfw_garbage_table_values(ch, tc, tei, count, 0); if (numdel != 0) { /* Run post-del hook to permit shrinking */ Modified: projects/ipfw/sys/netpfil/ipfw/ip_fw_table.h ============================================================================== --- projects/ipfw/sys/netpfil/ipfw/ip_fw_table.h Tue Sep 2 14:26:25 2014 (r270967) +++ projects/ipfw/sys/netpfil/ipfw/ip_fw_table.h Tue Sep 2 14:27:12 2014 (r270968) @@ -198,12 +198,13 @@ struct tableop_state; void ipfw_table_value_init(struct ip_fw_chain *ch); void ipfw_table_value_destroy(struct ip_fw_chain *ch); int ipfw_link_table_values(struct ip_fw_chain *ch, struct tableop_state *ts); -void ipfw_finalize_table_values(struct ip_fw_chain *ch, struct table_config *tc, +void ipfw_garbage_table_values(struct ip_fw_chain *ch, struct table_config *tc, struct tentry_info *tei, uint32_t count, int rollback); void ipfw_import_table_value_v1(ipfw_table_value *iv); void ipfw_export_table_value_v1(struct table_value *v, ipfw_table_value *iv); void ipfw_unref_table_values(struct ip_fw_chain *ch, struct table_config *tc, struct table_algo *ta, void *astate, struct table_info *ti); +void rollback_table_values(struct tableop_state *ts); int ipfw_rewrite_table_uidx(struct ip_fw_chain *chain, struct rule_check_info *ci); Modified: projects/ipfw/sys/netpfil/ipfw/ip_fw_table_value.c ============================================================================== --- projects/ipfw/sys/netpfil/ipfw/ip_fw_table_value.c Tue Sep 2 14:26:25 2014 (r270967) +++ projects/ipfw/sys/netpfil/ipfw/ip_fw_table_value.c Tue Sep 2 14:27:12 2014 (r270968) @@ -166,6 +166,9 @@ get_value_ptrs(struct ip_fw_chain *ch, s *pvi = vi; } +/* + * Update pointers to real vaues after @pval change. + */ static void update_tvalue(struct namedobj_instance *ni, struct named_object *no, void *arg) { @@ -184,9 +187,12 @@ update_tvalue(struct namedobj_instance * /* * Grows value storage shared among all tables. * Drops/reacquires UH locks. + * Notifies other running adds on @ch shared storage resize. + * Note function does not guarantee that free space + * will be available after invocation, so one caller needs + * to roll cycle himself. * - * Returns 0 on success. - * Note caller has to check @ts "modified" field. + * Returns 0 if case of no errors. */ static int resize_shared_value_storage(struct ip_fw_chain *ch) @@ -259,6 +265,10 @@ done: return (0); } +/* + * Drops reference for table value with index @kidx, stored in @pval and + * @vi. Frees value if it has no references. + */ static void unref_table_value(struct namedobj_instance *vi, struct table_value *pval, uint32_t kidx) @@ -339,21 +349,15 @@ ipfw_unref_table_values(struct ip_fw_cha * and set "modified" field to non-zero value to indicate * that we need to restart original operation. */ -static void -rollback_table_values(void *object, struct op_state *_state) +void +rollback_table_values(struct tableop_state *ts) { struct ip_fw_chain *ch; - struct tableop_state *ts; struct table_value *pval; struct tentry_info *ptei; struct namedobj_instance *vi; int i; - ts = (struct tableop_state *)_state; - - if (ts->tc != object && ts->ch != object) - return; - ch = ts->ch; IPFW_UH_WLOCK_ASSERT(ch); @@ -369,8 +373,6 @@ rollback_table_values(void *object, stru unref_table_value(vi, pval, ptei->value); } - - ts->modified = 1; } /* @@ -378,7 +380,6 @@ rollback_table_values(void *object, stru * Function may drop/reacquire UH lock. * * Returns 0 on success. - * Note that called has to check @ts "modified" value. */ static int alloc_table_vidx(struct ip_fw_chain *ch, struct tableop_state *ts, @@ -397,7 +398,7 @@ alloc_table_vidx(struct ip_fw_chain *ch, * lock/unlock, so we need to check "modified" * state. */ - rollback_table_values(ts->tc, &ts->opstate); + ts->opstate.func(ts->tc, &ts->opstate); error = resize_shared_value_storage(ch); return (error); /* ts->modified should be set, we will restart */ } @@ -428,11 +429,11 @@ alloc_table_vidx(struct ip_fw_chain *ch, } /* - * Drops value reference for unused values (updates, partially + * Drops value reference for unused values (updates, deletes, partially * successful adds or rollbacks). */ void -ipfw_finalize_table_values(struct ip_fw_chain *ch, struct table_config *tc, +ipfw_garbage_table_values(struct ip_fw_chain *ch, struct table_config *tc, struct tentry_info *tei, uint32_t count, int rollback) { int i; @@ -441,7 +442,7 @@ ipfw_finalize_table_values(struct ip_fw_ struct namedobj_instance *vi; /* - * We have two slightly different cases here: + * We have two slightly different ADD cases here: * either (1) we are successful / partially successful, * in that case we need * * to ignore ADDED entries values @@ -452,6 +453,8 @@ ipfw_finalize_table_values(struct ip_fw_ * (2): atomic rollback of partially successful operation * in that case we simply need to unref all entries. * + * DELETE case is simpler: no atomic support there, so + * we simply unref all non-zero values. */ /* @@ -482,6 +485,13 @@ ipfw_finalize_table_values(struct ip_fw_ } } +/* + * Main function used to link values of entries going to be added, + * to the index. Since we may perform many UH locks drops/acquires, + * handle changes by checking tablestate "modified" field. + * + * Success: return 0. + */ int ipfw_link_table_values(struct ip_fw_chain *ch, struct tableop_state *ts) { @@ -496,7 +506,7 @@ ipfw_link_table_values(struct ip_fw_chai /* * Stage 1: reference all existing values and - * save them inside the bitmask. + * save their indices. */ IPFW_UH_WLOCK_ASSERT(ch); get_value_ptrs(ch, ts->tc, ts->vshared, &pval, &vi); @@ -582,9 +592,10 @@ ipfw_link_table_values(struct ip_fw_chai /* May perform UH unlock/lock */ error = alloc_table_vidx(ch, ts, vi, &vidx); if (error != 0) { - rollback_table_values(tc, &ts->opstate); + ts->opstate.func(ts->tc, &ts->opstate); return (error); } + /* value storage resize has happened, return */ if (ts->modified != 0) return (0); @@ -625,6 +636,9 @@ ipfw_import_table_value_legacy(uint32_t v->limit = value; } +/* + * Export data to legacy table dumps opcodes. + */ uint32_t ipfw_export_table_value_legacy(struct table_value *v) { @@ -636,6 +650,10 @@ ipfw_export_table_value_legacy(struct ta return (v->tag); } +/* + * Imports table value from current userland format. + * Saves value in kernel format to the same place. + */ void ipfw_import_table_value_v1(ipfw_table_value *iv) { @@ -657,6 +675,10 @@ ipfw_import_table_value_v1(ipfw_table_va memcpy(iv, &v, sizeof(ipfw_table_value)); } +/* + * Export real table value @v to current userland format. + * Note that @v and @piv may point to the same memory. + */ void ipfw_export_table_value_v1(struct table_value *v, ipfw_table_value *piv) { @@ -678,6 +700,10 @@ ipfw_export_table_value_v1(struct table_ memcpy(piv, &iv, sizeof(iv)); } +/* + * Exports real value data into ipfw_table_value structure. + * Utilizes "spare1" field to store kernel index. + */ static void dump_tvalue(struct namedobj_instance *ni, struct named_object *no, void *arg) {