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

[Alexander V. Chernikov]

19.12.2019, 09:19, "Alexander V. Chernikov" <melifaro at 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);