[PATCH net-next 2/3] net/sched: act_vlan: size push_eth in get_fill_size

[Linkui Xiao]

From: Linkui Xiao <xiaolinkui@xxxxxxxxxx>

tcf_vlan_dump() emits TCA_VLAN_PUSH_ETH_DST and TCA_VLAN_PUSH_ETH_SRC for
every action installed with TCA_VLAN_ACT_PUSH_ETH, but
tcf_vlan_get_fill_size() budgets only TCA_VLAN_PARMS plus the push VLAN
id, protocol and priority.  Commit 19fbcb36a39e ("net/sched: act_vlan:
Add {POP,PUSH}_ETH actions") put the two Ethernet addresses in the dump
without putting them here.

get_fill_size() is the per-action term of the budget that
tcf_action_fill_size() returns to the core, which uses it to size the
netlink skb carrying the action, so it has to bound what dump() can write
rather than describe one particular instance.  Commit 4ba61bfd4998
("net/sched: add get_fill_size() to the five actions that lack one")
states the convention: "Size every optional attribute unconditionally".
act_vlan already follows it for TCA_VLAN_PUSH_VLAN_ID,
TCA_VLAN_PUSH_VLAN_PROTOCOL and TCA_VLAN_PUSH_VLAN_PRIORITY, all of which
are sized even though a PUSH_ETH instance emits none of them; the
push_eth attributes are simply missing.

This does not overflow today, and only by coincidence.  A PUSH_ETH dump
writes nla_total_size(sizeof(struct tc_vlan)) + 2 *
nla_total_size(ETH_ALEN) == 28 + 24 == 52 bytes, and the current budget
is also exactly 52 bytes, because the three PUSH_VLAN_* attributes that
a PUSH_ETH instance never emits happen to add up to the same 24 bytes the
two MAC addresses need.  That leaves zero slack, and nothing keeps it
that way: the moment any PUSH_VLAN_* attribute is made conditional --
precisely the change the convention quoted above forbids -- a push_eth
dump no longer fits the budget, and building the notification fails with
-EMSGSIZE.

Account for the two attributes the way the rest of the function already
does.  Nothing changes at runtime: the budget already equals or exceeds
what dump() writes for every instance, and all four consumers of it --
tcf_get_notify(), tcf_del_notify_msg(), tcf_add_notify_msg() and
tcf_reoffload_del_notify_msg() -- allocate at least
max(attr_size, NLMSG_GOODSIZE), which is SKB_WITH_OVERHEAD(PAGE_SIZE)
and dwarfs a 24-byte difference.  The RTM_GETACTION dump path sizes its
skb from the netlink dump machinery instead.

Tested by building with CONFIG_NET_ACT_VLAN=y; net/sched builds with W=1
and no new warnings.

Fixes: 19fbcb36a39e ("net/sched: act_vlan: Add {POP,PUSH}_ETH actions")
Signed-off-by: Linkui Xiao <xiaolinkui@xxxxxxxxxx>
---
 net/sched/act_vlan.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/sched/act_vlan.c b/net/sched/act_vlan.c
index 6a375fdc63dd..e666004bb919 100644
--- a/net/sched/act_vlan.c
+++ b/net/sched/act_vlan.c
@@ -348,7 +348,8 @@ static size_t tcf_vlan_get_fill_size(const struct tc_action *act)
 	return nla_total_size(sizeof(struct tc_vlan))
 		+ nla_total_size(sizeof(u16)) /* TCA_VLAN_PUSH_VLAN_ID */
 		+ nla_total_size(sizeof(u16)) /* TCA_VLAN_PUSH_VLAN_PROTOCOL */
-		+ nla_total_size(sizeof(u8)); /* TCA_VLAN_PUSH_VLAN_PRIORITY */
+		+ nla_total_size(sizeof(u8)) /* TCA_VLAN_PUSH_VLAN_PRIORITY */
+		+ 2 * nla_total_size(ETH_ALEN); /* TCA_VLAN_PUSH_ETH_{DST,SRC} */
 }
 
 static int tcf_vlan_offload_act_setup(struct tc_action *act, void *entry_data,
-- 
2.25.1