On Wed, Feb 27, 2019 at 9:41 AM Davide Caratti <dcara...@redhat.com> wrote: > +int tcf_action_check_ctrlact(int action, struct tcf_proto *tp, > + struct tcf_chain **handle,
Please use a better name than 'handle'. 'handle' is usually used for a hex numeric ID. Here you just want to save the allocated tcf_chain to this address. > + struct netlink_ext_ack *extack) > +{ > + int opcode = TC_ACT_EXT_OPCODE(action), ret = -EINVAL; > + u32 chain_index; > + > + if (!opcode) > + ret = action > TC_ACT_VALUE_MAX ? -EINVAL : 0; > + else if (opcode <= TC_ACT_EXT_OPCODE_MAX || action == TC_ACT_UNSPEC) > + ret = 0; > + if (ret) { > + NL_SET_ERR_MSG(extack, "invalid control action"); > + goto end; > + } > + > + if (TC_ACT_EXT_CMP(action, TC_ACT_GOTO_CHAIN)) { > + chain_index = action & TC_ACT_EXT_VAL_MASK; > + if (!tp) { > + ret = -EINVAL; > + NL_SET_ERR_MSG(extack, > + "can't use goto_chain with NULL > proto"); > + goto end; > + } > + if (!handle) { > + ret = -EINVAL; > + NL_SET_ERR_MSG(extack, > + "can't put goto_chain on NULL handle"); > + goto end; > + } > + *handle = tcf_chain_get_by_act(tp->chain->block, chain_index); > + if (!*handle) { > + ret = -ENOMEM; Is -ENOMEM okay here? I feel like it should be -ENOSPC or whatever tcf_chain_get_by_act() says. Thanks.