| 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: <54A45819.2090206@gmail.com>
Date: Wed, 31 Dec 2014 12:10:01 -0800
From: John Fastabend <john.fastabend@...il.com>
To: tgraf@...g.ch, sfeldma@...il.com, jiri@...nulli.us,
jhs@...atatu.com, simon.horman@...ronome.com
CC: netdev@...r.kernel.org, davem@...emloft.net, andy@...yhouse.net
Subject: Re: [net-next PATCH v1 01/11] net: flow_table: create interface for
hw match/action tables
On 12/31/2014 11:45 AM, John Fastabend wrote:
> Currently, we do not have an interface to query hardware and learn
> the capabilities of the device. This makes it very difficult to use
> hardware flow tables.
>
oops missed a few dev_put calls so at least need a new rev
for this. I'll wait a few days for feedback though.
[...]
> +
> +static int net_flow_cmd_get_actions(struct sk_buff *skb,
> + struct genl_info *info)
> +{
> + struct net_flow_action **a;
> + struct net_device *dev;
> + struct sk_buff *msg;
> +
> + dev = net_flow_get_dev(info);
> + if (!dev)
> + return -EINVAL;
> +
> + if (!dev->netdev_ops->ndo_flow_get_actions) {
> + dev_put(dev);
> + return -EOPNOTSUPP;
> + }
> +
> + a = dev->netdev_ops->ndo_flow_get_actions(dev);
> + if (!a)
missing dev_put(dev) here.
> + return -EBUSY;
> +
> + msg = net_flow_build_actions_msg(a, dev,
> + info->snd_portid,
> + info->snd_seq,
> + NET_FLOW_TABLE_CMD_GET_ACTIONS);
> + dev_put(dev);
> +
> + if (IS_ERR(msg))
> + return PTR_ERR(msg);
> +
> + return genlmsg_reply(msg, info);
> +}
> +
> +static int net_flow_put_table(struct net_device *dev,
> + struct sk_buff *skb,
> + struct net_flow_table *t)
> +{
> + struct nlattr *matches, *actions;
> + int i;
> +
> + if (nla_put_string(skb, NET_FLOW_TABLE_ATTR_NAME, t->name) ||
> + nla_put_u32(skb, NET_FLOW_TABLE_ATTR_UID, t->uid) ||
> + nla_put_u32(skb, NET_FLOW_TABLE_ATTR_SOURCE, t->source) ||
> + nla_put_u32(skb, NET_FLOW_TABLE_ATTR_SIZE, t->size))
> + return -EMSGSIZE;
> +
> + matches = nla_nest_start(skb, NET_FLOW_TABLE_ATTR_MATCHES);
> + if (!matches)
> + return -EMSGSIZE;
> +
> + for (i = 0; t->matches[i].instance; i++)
> + nla_put(skb, NET_FLOW_FIELD_REF,
> + sizeof(struct net_flow_field_ref),
> + &t->matches[i]);
need to check the return codes here.
> + nla_nest_end(skb, matches);
> +
> + actions = nla_nest_start(skb, NET_FLOW_TABLE_ATTR_ACTIONS);
> + if (!actions)
> + return -EMSGSIZE;
> +
> + for (i = 0; t->actions[i]; i++) {
> + if (nla_put_u32(skb,
> + NET_FLOW_ACTION_ATTR_UID,
> + t->actions[i])) {
> + nla_nest_cancel(skb, actions);
> + return -EMSGSIZE;
> + }
remembered to do the check here though ;)
> + }
> + nla_nest_end(skb, actions);
> +
> + return 0;
> +}
> +
[...]
> +
> +static struct sk_buff *net_flow_build_tables_msg(struct net_flow_table **t,
> + struct net_device *dev,
> + u32 portid, int seq, u8 cmd)
> +{
> + struct genlmsghdr *hdr;
> + struct sk_buff *skb;
> + int err = -ENOBUFS;
> +
> + skb = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
> + if (!skb)
> + return ERR_PTR(-ENOBUFS);
> +
> + hdr = genlmsg_put(skb, portid, seq, &net_flow_nl_family, 0, cmd);
> + if (!hdr)
> + goto out;
> +
> + if (nla_put_u32(skb,
> + NET_FLOW_IDENTIFIER_TYPE,
> + NET_FLOW_IDENTIFIER_IFINDEX) ||
> + nla_put_u32(skb, NET_FLOW_IDENTIFIER, dev->ifindex)) {
> + err = -ENOBUFS;
> + goto out;
> + }
> +
> + err = net_flow_put_tables(dev, skb, t);
> + if (err < 0)
> + goto out;
> +
> + err = genlmsg_end(skb, hdr);
> + if (err < 0)
> + goto out;
> +
> + return skb;
> +out:
> + nlmsg_free(skb);
> + return ERR_PTR(err);
> +}
> +
> +static int net_flow_cmd_get_tables(struct sk_buff *skb,
> + struct genl_info *info)
> +{
> + struct net_flow_table **tables;
> + struct net_device *dev;
> + struct sk_buff *msg;
> +
> + dev = net_flow_get_dev(info);
> + if (!dev)
> + return -EINVAL;
> +
> + if (!dev->netdev_ops->ndo_flow_get_tables) {
> + dev_put(dev);
> + return -EOPNOTSUPP;
> + }
> +
> + tables = dev->netdev_ops->ndo_flow_get_tables(dev);
> + if (!tables) /* transient failure should always have some table */
need dev_put()
> + return -EBUSY;
> +
> + msg = net_flow_build_tables_msg(tables, dev,
> + info->snd_portid,
> + info->snd_seq,
> + NET_FLOW_TABLE_CMD_GET_TABLES);
> + dev_put(dev);
> +
> + if (IS_ERR(msg))
> + return PTR_ERR(msg);
> +
> + return genlmsg_reply(msg, info);
> +}
> +
[...]
> +
> +static int net_flow_put_headers(struct sk_buff *skb,
> + struct net_flow_header **headers)
> +{
> + struct nlattr *nest, *hdr, *fields;
> + struct net_flow_header *h;
> + int i, err;
> +
> + nest = nla_nest_start(skb, NET_FLOW_HEADERS);
> + if (!nest)
> + return -EMSGSIZE;
> +
> + for (i = 0; headers[i]->uid; i++) {
> + err = -EMSGSIZE;
> + h = headers[i];
> +
> + hdr = nla_nest_start(skb, NET_FLOW_HEADER);
> + if (!hdr)
> + goto hdr_put_failure;
> +
> + if (nla_put_string(skb, NET_FLOW_HEADER_ATTR_NAME, h->name) ||
> + nla_put_u32(skb, NET_FLOW_HEADER_ATTR_UID, h->uid))
> + goto attr_put_failure;
> +
> + fields = nla_nest_start(skb, NET_FLOW_HEADER_ATTR_FIELDS);
> + if (!fields)
> + goto attr_put_failure;
> +
> + err = net_flow_put_fields(skb, h);
> + if (err)
> + goto fields_put_failure;
> +
> + nla_nest_end(skb, fields);
> +
can remove this new line I think it doesn't add much.
> + nla_nest_end(skb, hdr);
> + }
> + nla_nest_end(skb, nest);
> +
> + return 0;
> +fields_put_failure:
> + nla_nest_cancel(skb, fields);
> +attr_put_failure:
> + nla_nest_cancel(skb, hdr);
> +hdr_put_failure:
> + nla_nest_cancel(skb, nest);
> + return err;
> +}
> +
[...]
> +
> +static int net_flow_cmd_get_headers(struct sk_buff *skb,
> + struct genl_info *info)
> +{
> + struct net_flow_header **h;
> + struct net_device *dev;
> + struct sk_buff *msg;
> +
> + dev = net_flow_get_dev(info);
> + if (!dev)
> + return -EINVAL;
> +
> + if (!dev->netdev_ops->ndo_flow_get_headers) {
> + dev_put(dev);
> + return -EOPNOTSUPP;
> + }
> +
> + h = dev->netdev_ops->ndo_flow_get_headers(dev);
> + if (!h)
dev_put again
> + return -EBUSY;
> +
> + msg = net_flow_build_headers_msg(h, dev,
> + info->snd_portid,
> + info->snd_seq,
> + NET_FLOW_TABLE_CMD_GET_HEADERS);
> + dev_put(dev);
> +
> + if (IS_ERR(msg))
> + return PTR_ERR(msg);
> +
> + return genlmsg_reply(msg, info);
> +}
> +
[...]
> +
> +static int net_flow_cmd_get_header_graph(struct sk_buff *skb,
> + struct genl_info *info)
> +{
> + struct net_flow_hdr_node **h;
> + struct net_device *dev;
> + struct sk_buff *msg;
> +
> + dev = net_flow_get_dev(info);
> + if (!dev)
> + return -EINVAL;
> +
> + if (!dev->netdev_ops->ndo_flow_get_hdr_graph) {
> + dev_put(dev);
> + return -EOPNOTSUPP;
> + }
> +
> + h = dev->netdev_ops->ndo_flow_get_hdr_graph(dev);
> + if (!h)
dev_put() seems I copy/pasted the same template for each cmd.
> + return -EBUSY;
> +
> + msg = net_flow_build_header_graph_msg(h, dev,
> + info->snd_portid,
> + info->snd_seq,
> + NET_FLOW_TABLE_CMD_GET_HDR_GRAPH);
> + dev_put(dev);
> +
> + if (IS_ERR(msg))
> + return PTR_ERR(msg);
> +
> + return genlmsg_reply(msg, info);
> +}
> +
[...]
> +
> +static int net_flow_cmd_get_table_graph(struct sk_buff *skb,
> + struct genl_info *info)
> +{
> + struct net_flow_tbl_node **g;
> + struct net_device *dev;
> + struct sk_buff *msg;
> +
> + dev = net_flow_get_dev(info);
> + if (!dev)
> + return -EINVAL;
> +
> + if (!dev->netdev_ops->ndo_flow_get_tbl_graph) {
> + dev_put(dev);
> + return -EOPNOTSUPP;
> + }
> +
> + g = dev->netdev_ops->ndo_flow_get_tbl_graph(dev);
> + if (!g)
dev_put
> + return -EBUSY;
> +
[...]
--
John Fastabend Intel Corporation
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists