| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAM_iQpW-qjM5H00sUbPhrimJ2aPDAiO=bTVuF88bCtPH=qz4gQ@mail.gmail.com>
Date: Fri, 13 Mar 2020 11:36:54 -0700
From: Cong Wang <xiyou.wangcong@...il.com>
To: Po Liu <Po.Liu@....com>
Cc: David Miller <davem@...emloft.net>,
LKML <linux-kernel@...r.kernel.org>,
Linux Kernel Network Developers <netdev@...r.kernel.org>,
Vinicius Costa Gomes <vinicius.gomes@...el.com>,
claudiu.manoil@....com, vladimir.oltean@....com,
alexandru.marginean@....com, xiaoliang.yang_1@....com,
roy.zang@....com, mingkai.hu@....com, jerry.huang@....com,
leoyang.li@....com, michael.chan@...adcom.com, vishal@...lsio.com,
Saeed Mahameed <saeedm@...lanox.com>,
Leon Romanovsky <leon@...nel.org>,
Jiri Pirko <jiri@...lanox.com>,
Ido Schimmel <idosch@...lanox.com>,
alexandre.belloni@...tlin.com, UNGLinuxDriver@...rochip.com,
Jakub Kicinski <kuba@...nel.org>,
Jamal Hadi Salim <jhs@...atatu.com>,
John Hurley <john.hurley@...ronome.com>,
Simon Horman <simon.horman@...ronome.com>,
Pieter Jansen van Vuuren
<pieter.jansenvanvuuren@...ronome.com>,
Pablo Neira Ayuso <pablo@...filter.org>, moshe@...lanox.com,
ivan.khoronzhuk@...aro.org, Murali Karicheri <m-karicheri2@...com>,
andre.guedes@...ux.intel.com,
Jakub Kicinski <jakub.kicinski@...ronome.com>
Subject: Re: [RFC,net-next 2/9] net: qos: introduce a gate control flow action
On Fri, Mar 6, 2020 at 5:14 AM Po Liu <Po.Liu@....com> wrote:
> +static int parse_gate_list(struct nlattr *list,
> + struct tcf_gate_params *sched,
> + struct netlink_ext_ack *extack)
> +{
> + struct nlattr *n;
> + int err, rem;
> + int i = 0;
> +
> + if (!list)
> + return -EINVAL;
> +
> + nla_for_each_nested(n, list, rem) {
> + struct tcfg_gate_entry *entry;
> +
> + if (nla_type(n) != TCA_GATE_ONE_ENTRY) {
> + NL_SET_ERR_MSG(extack, "Attribute isn't type 'entry'");
> + continue;
> + }
> +
> + entry = kzalloc(sizeof(*entry), GFP_KERNEL);
> + if (!entry) {
> + NL_SET_ERR_MSG(extack, "Not enough memory for entry");
> + return -ENOMEM;
> + }
You need to free all previous allocated entries on this list
when an error happens, right?
> +
> + err = parse_gate_entry(n, entry, i, extack);
> + if (err < 0) {
> + kfree(entry);
> + return err;
> + }
> +
> + list_add_tail(&entry->list, &sched->entries);
> + i++;
> + }
> +
> + sched->num_entries = i;
> +
> + return i;
> +}
> +
> +static int tcf_gate_init(struct net *net, struct nlattr *nla,
> + struct nlattr *est, struct tc_action **a,
> + int ovr, int bind, bool rtnl_held,
> + struct tcf_proto *tp, u32 flags,
> + struct netlink_ext_ack *extack)
> +{
> + struct tc_action_net *tn = net_generic(net, gate_net_id);
> + enum tk_offsets tk_offset = TK_OFFS_TAI;
> + struct nlattr *tb[TCA_GATE_MAX + 1];
> + struct tcf_chain *goto_ch = NULL;
> + struct tcfg_gate_entry *next;
> + struct tcf_gate_params *p;
> + struct gate_action *gact;
> + s32 clockid = CLOCK_TAI;
> + struct tc_gate *parm;
> + struct tcf_gate *g;
> + int ret = 0, err;
> + u64 basetime = 0;
> + u32 gflags = 0;
> + s32 prio = -1;
> + ktime_t start;
> + u32 index;
> +
> + if (!nla)
> + return -EINVAL;
> +
> + err = nla_parse_nested_deprecated(tb, TCA_GATE_MAX,
> + nla, gate_policy, NULL);
> + if (err < 0)
> + return err;
> +
> + if (!tb[TCA_GATE_PARMS])
> + return -EINVAL;
> + parm = nla_data(tb[TCA_GATE_PARMS]);
> + index = parm->index;
> + err = tcf_idr_check_alloc(tn, &index, a, bind);
> + if (err < 0)
> + return err;
> +
> + if (err && bind)
> + return 0;
> +
> + if (!err) {
> + ret = tcf_idr_create_from_flags(tn, index, est, a,
> + &act_gate_ops, bind, flags);
> + if (ret) {
> + tcf_idr_cleanup(tn, index);
> + return ret;
> + }
> +
> + ret = ACT_P_CREATED;
> + } else if (!ovr) {
> + tcf_idr_release(*a, bind);
> + return -EEXIST;
> + }
> +
> + if (tb[TCA_GATE_PRIORITY])
> + prio = nla_get_s32(tb[TCA_GATE_PRIORITY]);
> +
> + if (tb[TCA_GATE_BASE_TIME])
> + basetime = nla_get_u64(tb[TCA_GATE_BASE_TIME]);
> +
> + if (tb[TCA_GATE_FLAGS])
> + gflags = nla_get_u32(tb[TCA_GATE_FLAGS]);
> +
> + if (tb[TCA_GATE_CLOCKID]) {
> + clockid = nla_get_s32(tb[TCA_GATE_CLOCKID]);
> + switch (clockid) {
> + case CLOCK_REALTIME:
> + tk_offset = TK_OFFS_REAL;
> + break;
> + case CLOCK_MONOTONIC:
> + tk_offset = TK_OFFS_MAX;
> + break;
> + case CLOCK_BOOTTIME:
> + tk_offset = TK_OFFS_BOOT;
> + break;
> + case CLOCK_TAI:
> + tk_offset = TK_OFFS_TAI;
> + break;
> + default:
> + NL_SET_ERR_MSG(extack, "Invalid 'clockid'");
> + goto release_idr;
> + }
> + }
> +
> + err = tcf_action_check_ctrlact(parm->action, tp, &goto_ch, extack);
> + if (err < 0)
> + goto release_idr;
> +
> + g = to_gate(*a);
> +
> + gact = kzalloc(sizeof(*gact), GFP_KERNEL);
> + if (!gact) {
> + err = -ENOMEM;
> + goto put_chain;
> + }
> +
> + p = get_gate_param(gact);
> +
> + INIT_LIST_HEAD(&p->entries);
> + if (tb[TCA_GATE_ENTRY_LIST]) {
> + err = parse_gate_list(tb[TCA_GATE_ENTRY_LIST], p, extack);
> + if (err < 0)
> + goto release_mem;
> + }
> +
> + if (tb[TCA_GATE_CYCLE_TIME]) {
> + p->tcfg_cycletime = nla_get_u64(tb[TCA_GATE_CYCLE_TIME]);
> + } else {
> + struct tcfg_gate_entry *entry;
> + ktime_t cycle = 0;
> +
> + list_for_each_entry(entry, &p->entries, list)
> + cycle = ktime_add_ns(cycle, entry->interval);
> + p->tcfg_cycletime = cycle;
> + }
> +
> + if (tb[TCA_GATE_CYCLE_TIME_EXT])
> + p->tcfg_cycletime_ext =
> + nla_get_u64(tb[TCA_GATE_CYCLE_TIME_EXT]);
> +
> + p->tcfg_priority = prio;
> + p->tcfg_basetime = basetime;
> + p->tcfg_clockid = clockid;
> + p->tcfg_flags = gflags;
> +
> + gact->tk_offset = tk_offset;
> + spin_lock_init(&gact->entry_lock);
> + hrtimer_init(&gact->hitimer, clockid, HRTIMER_MODE_ABS);
> + gact->hitimer.function = gate_timer_func;
> +
> + err = gate_get_start_time(gact, &start);
> + if (err < 0) {
> + NL_SET_ERR_MSG(extack,
> + "Internal error: failed get start time");
> + goto release_mem;
> + }
> +
> + gact->current_close_time = start;
> + gact->current_gate_status = GATE_ACT_GATE_OPEN | GATE_ACT_PENDING;
> +
> + next = list_first_entry(&p->entries, struct tcfg_gate_entry, list);
> + rcu_assign_pointer(gact->next_entry, next);
> +
> + gate_start_timer(gact, start);
> +
> + spin_lock_bh(&g->tcf_lock);
> + goto_ch = tcf_action_set_ctrlact(*a, parm->action, goto_ch);
> + gact = rcu_replace_pointer(g->actg, gact,
> + lockdep_is_held(&g->tcf_lock));
> + spin_unlock_bh(&g->tcf_lock);
> +
> + if (goto_ch)
> + tcf_chain_put_by_act(goto_ch);
> + if (gact)
> + kfree_rcu(gact, rcu);
> +
> + if (ret == ACT_P_CREATED)
> + tcf_idr_insert(tn, *a);
> + return ret;
> +
> +release_mem:
> + kfree_rcu(gact, rcu);
No need to bother RCU for an error path, right?
Thanks.
Powered by blists - more mailing lists