Re: [PATCH v2 net-next 3/4] net sched actions: calculate add/delete event message size

2018-03-07 Thread Roman Mashak
David Miller  writes:

> From: Roman Mashak 
> Date: Tue,  6 Mar 2018 16:55:23 -0500
>
>> +static size_t tcf_action_fill_size(const struct tc_action *act)
>> +{
>> +if (act->ops->get_fill_size)
>> +return act->ops->get_fill_size(act) +
>> +tcf_action_shared_attrs_size(act);
>> +return 0;
>> +}
>
> I don't understand this.
>
> The shared attrs should be considered regardless of whether an action
> type specific ->get_fill_size() is implemented.
>
> But instead, you return zero in that case.

Actually when action  specific ->get_fill_size() is not
implemented, I want to fall back to the current behaviour, where we
always allocate NLMSG_GOODSIZE bytes for skb. So in this case size
passed to tcf_add_notify is estimated in tcf_action_full_attrs_size()
and smaller then NLMSG_GOODSIZE (which is page size).

But may be you're right, API should return explicit size of the message,
we can't expect PAGE_SIZE to be large enough.

I will then change this in v3.


Re: [PATCH v2 net-next 3/4] net sched actions: calculate add/delete event message size

2018-03-07 Thread David Miller
From: Roman Mashak 
Date: Tue,  6 Mar 2018 16:55:23 -0500

> +static size_t tcf_action_fill_size(const struct tc_action *act)
> +{
> + if (act->ops->get_fill_size)
> + return act->ops->get_fill_size(act) +
> + tcf_action_shared_attrs_size(act);
> + return 0;
> +}

I don't understand this.

The shared attrs should be considered regardless of whether an action
type specific ->get_fill_size() is implemented.

But instead, you return zero in that case.


[PATCH v2 net-next 3/4] net sched actions: calculate add/delete event message size

2018-03-06 Thread Roman Mashak
Introduce routines to calculate size of the shared tc netlink attributes
and the full message size including netlink header and tc service header.

Update add/delete action logic to have the size for event messages,
the size is passed to tcf_add_notify() and tcf_del_notify() where the
notification message is being allocated and constructed.

Signed-off-by: Roman Mashak 
---
 net/sched/act_api.c | 42 ++
 1 file changed, 42 insertions(+)

diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index 3de0e06..8809e48 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -109,6 +109,41 @@ int __tcf_idr_release(struct tc_action *p, bool bind, bool 
strict)
 }
 EXPORT_SYMBOL(__tcf_idr_release);
 
+static size_t tcf_action_shared_attrs_size(const struct tc_action *act)
+{
+   u32 cookie_len = 0;
+
+   if (act->act_cookie)
+   cookie_len = nla_total_size(act->act_cookie->len);
+
+   return  nla_total_size(0) /* action number nested */
+   + nla_total_size(IFNAMSIZ) /* TCA_ACT_KIND */
+   + cookie_len /* TCA_ACT_COOKIE */
+   + nla_total_size(0) /* TCA_ACT_STATS nested */
+   /* TCA_STATS_BASIC */
+   + nla_total_size_64bit(sizeof(struct gnet_stats_basic))
+   /* TCA_STATS_QUEUE */
+   + nla_total_size_64bit(sizeof(struct gnet_stats_queue))
+   + nla_total_size(0) /* TCA_OPTIONS nested */
+   + nla_total_size(sizeof(struct tcf_t)); /* TCA_GACT_TM */
+}
+
+static size_t tcf_action_full_attrs_size(size_t sz)
+{
+   return NLMSG_HDRLEN /* struct nlmsghdr */
+   + sizeof(struct tcamsg)
+   + nla_total_size(0) /* TCA_ACT_TAB nested */
+   + sz;
+}
+
+static size_t tcf_action_fill_size(const struct tc_action *act)
+{
+   if (act->ops->get_fill_size)
+   return act->ops->get_fill_size(act) +
+   tcf_action_shared_attrs_size(act);
+   return 0;
+}
+
 static int tcf_dump_walker(struct tcf_idrinfo *idrinfo, struct sk_buff *skb,
   struct netlink_callback *cb)
 {
@@ -746,6 +781,7 @@ int tcf_action_init(struct net *net, struct tcf_proto *tp, 
struct nlattr *nla,
 {
struct nlattr *tb[TCA_ACT_MAX_PRIO + 1];
struct tc_action *act;
+   size_t sz = 0;
int err;
int i;
 
@@ -761,11 +797,14 @@ int tcf_action_init(struct net *net, struct tcf_proto 
*tp, struct nlattr *nla,
goto err;
}
act->order = i;
+   sz += tcf_action_fill_size(act);
if (ovr)
act->tcfa_refcnt++;
list_add_tail(>list, actions);
}
 
+   *attr_size = tcf_action_full_attrs_size(sz);
+
/* Remove the temp refcnt which was necessary to protect against
 * destroying an existing action which was being replaced
 */
@@ -1056,9 +1095,12 @@ tca_action_gd(struct net *net, struct nlattr *nla, 
struct nlmsghdr *n,
goto err;
}
act->order = i;
+   attr_size += tcf_action_fill_size(act);
list_add_tail(>list, );
}
 
+   attr_size = tcf_action_full_attrs_size(attr_size);
+
if (event == RTM_GETACTION)
ret = tcf_get_notify(net, portid, n, , event, extack);
else { /* delete */
-- 
2.7.4