Date: Thu, 19 Dec 2019 09:26:57 +0000 From: Alexander V. Chernikov <melifaro@ipfw.ru> To: "src-committers@freebsd.org" <src-committers@freebsd.org>, "svn-src-all@freebsd.org" <svn-src-all@freebsd.org>, "svn-src-head@freebsd.org" <svn-src-head@freebsd.org> Subject: Re: svn commit: r355908 - head/sys/netpfil/ipfw Message-ID: <38008821576747617@vla3-985002032993.qloud-c.yandex.net> In-Reply-To: <201912190919.xBJ9JSXS014836@repo.freebsd.org> References: <201912190919.xBJ9JSXS014836@repo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
19.12.2019, 09:19, "Alexander V. Chernikov" <melifaro@freebsd.org>: > Author: melifaro > Date: Thu Dec 19 09:19:27 2019 > New Revision: 355908 > URL: https://svnweb.freebsd.org/changeset/base/355908 > > Log: > svn-commit.tmp Should have been ipfw: Don't rollback state in alloc_table_vidx() if atomicity is not required. Submitted by: Neel Chauhan <neel AT neelc DOT org> MFC after: 2 weeks Differential Revision: https://reviews.freebsd.org/D22662 > > Modified: > head/sys/netpfil/ipfw/ip_fw_table.c > head/sys/netpfil/ipfw/ip_fw_table.h > head/sys/netpfil/ipfw/ip_fw_table_value.c > > Modified: head/sys/netpfil/ipfw/ip_fw_table.c > ============================================================================== > --- head/sys/netpfil/ipfw/ip_fw_table.c Thu Dec 19 08:52:16 2019 (r355907) > +++ head/sys/netpfil/ipfw/ip_fw_table.c Thu Dec 19 09:19:27 2019 (r355908) > @@ -623,7 +623,7 @@ restart: > * > * May release/reacquire UH_WLOCK. > */ > - error = ipfw_link_table_values(ch, &ts); > + error = ipfw_link_table_values(ch, &ts, flags); > if (error != 0) > goto cleanup; > if (ts.modified != 0) > @@ -654,6 +654,14 @@ restart: > num = 0; > /* check limit before adding */ > if ((error = check_table_limit(tc, ptei)) == 0) { > + /* > + * It should be safe to insert a record w/o > + * a properly-linked value if atomicity is > + * not required. > + * > + * If the added item does not have a valid value > + * index, it would get rejected by ta->add(). > + * */ > error = ta->add(tc->astate, KIDX_TO_TI(ch, kidx), > ptei, v, &num); > /* Set status flag to inform userland */ > > Modified: head/sys/netpfil/ipfw/ip_fw_table.h > ============================================================================== > --- head/sys/netpfil/ipfw/ip_fw_table.h Thu Dec 19 08:52:16 2019 (r355907) > +++ head/sys/netpfil/ipfw/ip_fw_table.h Thu Dec 19 09:19:27 2019 (r355908) > @@ -168,7 +168,8 @@ struct table_config; > struct tableop_state; > void ipfw_table_value_init(struct ip_fw_chain *ch, int first); > void ipfw_table_value_destroy(struct ip_fw_chain *ch, int last); > -int ipfw_link_table_values(struct ip_fw_chain *ch, struct tableop_state *ts); > +int ipfw_link_table_values(struct ip_fw_chain *ch, struct tableop_state *ts, > + uint8_t flags); > 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); > > Modified: head/sys/netpfil/ipfw/ip_fw_table_value.c > ============================================================================== > --- head/sys/netpfil/ipfw/ip_fw_table_value.c Thu Dec 19 08:52:16 2019 (r355907) > +++ head/sys/netpfil/ipfw/ip_fw_table_value.c Thu Dec 19 09:19:27 2019 (r355908) > @@ -363,7 +363,7 @@ rollback_table_values(struct tableop_state *ts) > */ > static int > alloc_table_vidx(struct ip_fw_chain *ch, struct tableop_state *ts, > - struct namedobj_instance *vi, uint16_t *pvidx) > + struct namedobj_instance *vi, uint16_t *pvidx, uint8_t flags) > { > int error, vlimit; > uint16_t vidx; > @@ -384,16 +384,13 @@ alloc_table_vidx(struct ip_fw_chain *ch, struct tableo > } > > vlimit = ts->ta->vlimit; > - if (vlimit != 0 && vidx >= vlimit) { > + if (vlimit != 0 && vidx >= vlimit && !(flags & IPFW_CTF_ATOMIC)) { > > /* > * Algorithm is not able to store given index. > * We have to rollback state, start using > * per-table value array or return error > * if we're already using it. > - * > - * TODO: do not rollback state if > - * atomicity is not required. > */ > if (ts->vshared != 0) { > /* shared -> per-table */ > @@ -426,9 +423,10 @@ ipfw_garbage_table_values(struct ip_fw_chain *ch, stru > * either (1) we are successful / partially successful, > * in that case we need > * * to ignore ADDED entries values > - * * rollback every other values (either UPDATED since > - * old value has been stored there, or some failure like > - * EXISTS or LIMIT or simply "ignored" case. > + * * rollback every other values if atomicity is not > + * * required (either UPDATED since old value has been > + * stored there, or some failure like EXISTS or LIMIT > + * or simply "ignored" case. > * > * (2): atomic rollback of partially successful operation > * in that case we simply need to unref all entries. > @@ -473,7 +471,8 @@ ipfw_garbage_table_values(struct ip_fw_chain *ch, stru > * Success: return 0. > */ > int > -ipfw_link_table_values(struct ip_fw_chain *ch, struct tableop_state *ts) > +ipfw_link_table_values(struct ip_fw_chain *ch, struct tableop_state *ts, > + uint8_t flags) > { > int error, i, found; > struct namedobj_instance *vi; > @@ -577,7 +576,7 @@ ipfw_link_table_values(struct ip_fw_chain *ch, struct > } > > /* May perform UH unlock/lock */ > - error = alloc_table_vidx(ch, ts, vi, &vidx); > + error = alloc_table_vidx(ch, ts, vi, &vidx, flags); > if (error != 0) { > ts->opstate.func(ts->tc, &ts->opstate); > return (error);
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?38008821576747617>