1 From dc452a471dbae8aca8257c565174212620880093 Mon Sep 17 00:00:00 2001
2 From: Vladimir Oltean <vladimir.oltean@nxp.com>
3 Date: Fri, 10 Dec 2021 01:34:37 +0200
4 Subject: net: dsa: introduce tagger-owned storage for private and shared data
6 Ansuel is working on register access over Ethernet for the qca8k switch
7 family. This requires the qca8k tagging protocol driver to receive
8 frames which aren't intended for the network stack, but instead for the
9 qca8k switch driver itself.
11 The dp->priv is currently the prevailing method for passing data back
12 and forth between the tagging protocol driver and the switch driver.
13 However, this method is riddled with caveats.
15 The DSA design allows in principle for any switch driver to return any
16 protocol it desires in ->get_tag_protocol(). The dsa_loop driver can be
17 modified to do just that. But in the current design, the memory behind
18 dp->priv has to be allocated by the switch driver, so if the tagging
19 protocol is paired to an unexpected switch driver, we may end up in NULL
20 pointer dereferences inside the kernel, or worse (a switch driver may
21 allocate dp->priv according to the expectations of a different tagger).
23 The latter possibility is even more plausible considering that DSA
24 switches can dynamically change tagging protocols in certain cases
25 (dsa <-> edsa, ocelot <-> ocelot-8021q), and the current design lends
26 itself to mistakes that are all too easy to make.
28 This patch proposes that the tagging protocol driver should manage its
29 own memory, instead of relying on the switch driver to do so.
30 After analyzing the different in-tree needs, it can be observed that the
31 required tagger storage is per switch, therefore a ds->tagger_data
32 pointer is introduced. In principle, per-port storage could also be
33 introduced, although there is no need for it at the moment. Future
34 changes will replace the current usage of dp->priv with ds->tagger_data.
36 We define a "binding" event between the DSA switch tree and the tagging
37 protocol. During this binding event, the tagging protocol's ->connect()
38 method is called first, and this may allocate some memory for each
39 switch of the tree. Then a cross-chip notifier is emitted for the
40 switches within that tree, and they are given the opportunity to fix up
41 the tagger's memory (for example, they might set up some function
42 pointers that represent virtual methods for consuming packets).
43 Because the memory is owned by the tagger, there exists a ->disconnect()
44 method for the tagger (which is the place to free the resources), but
45 there doesn't exist a ->disconnect() method for the switch driver.
46 This is part of the design. The switch driver should make minimal use of
47 the public part of the tagger data, and only after type-checking it
48 using the supplied "proto" argument.
50 In the code there are in fact two binding events, one is the initial
51 event in dsa_switch_setup_tag_protocol(). At this stage, the cross chip
52 notifier chains aren't initialized, so we call each switch's connect()
53 method by hand. Then there is dsa_tree_bind_tag_proto() during
54 dsa_tree_change_tag_proto(), and here we have an old protocol and a new
55 one. We first connect to the new one before disconnecting from the old
56 one, to simplify error handling a bit and to ensure we remain in a valid
59 Co-developed-by: Ansuel Smith <ansuelsmth@gmail.com>
60 Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
61 Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
62 Signed-off-by: David S. Miller <davem@davemloft.net>
64 include/net/dsa.h | 12 +++++++++
65 net/dsa/dsa2.c | 73 +++++++++++++++++++++++++++++++++++++++++++++++++++---
66 net/dsa/dsa_priv.h | 1 +
67 net/dsa/switch.c | 14 +++++++++++
68 4 files changed, 96 insertions(+), 4 deletions(-)
70 diff --git a/include/net/dsa.h b/include/net/dsa.h
71 index bdf308a5c55ee..8b496c7e62ef8 100644
72 --- a/include/net/dsa.h
73 +++ b/include/net/dsa.h
74 @@ -82,12 +82,15 @@ enum dsa_tag_protocol {
78 +struct dsa_switch_tree;
80 struct dsa_device_ops {
81 struct sk_buff *(*xmit)(struct sk_buff *skb, struct net_device *dev);
82 struct sk_buff *(*rcv)(struct sk_buff *skb, struct net_device *dev);
83 void (*flow_dissect)(const struct sk_buff *skb, __be16 *proto,
85 + int (*connect)(struct dsa_switch_tree *dst);
86 + void (*disconnect)(struct dsa_switch_tree *dst);
87 unsigned int needed_headroom;
88 unsigned int needed_tailroom;
90 @@ -337,6 +340,8 @@ struct dsa_switch {
97 * Configuration data for this switch.
99 @@ -689,6 +694,13 @@ struct dsa_switch_ops {
100 enum dsa_tag_protocol mprot);
101 int (*change_tag_protocol)(struct dsa_switch *ds, int port,
102 enum dsa_tag_protocol proto);
104 + * Method for switch drivers to connect to the tagging protocol driver
105 + * in current use. The switch driver can provide handlers for certain
106 + * types of packets for switch management.
108 + int (*connect_tag_protocol)(struct dsa_switch *ds,
109 + enum dsa_tag_protocol proto);
111 /* Optional switch-wide initialization and destruction methods */
112 int (*setup)(struct dsa_switch *ds);
113 diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
114 index 8814fa0e44c84..cf65661686209 100644
117 @@ -248,8 +248,12 @@ static struct dsa_switch_tree *dsa_tree_alloc(int index)
119 static void dsa_tree_free(struct dsa_switch_tree *dst)
122 + if (dst->tag_ops) {
123 + if (dst->tag_ops->disconnect)
124 + dst->tag_ops->disconnect(dst);
126 dsa_tag_driver_put(dst->tag_ops);
128 list_del(&dst->list);
131 @@ -822,7 +826,7 @@ static int dsa_switch_setup_tag_protocol(struct dsa_switch *ds)
134 if (tag_ops->proto == dst->default_proto)
138 dsa_switch_for_each_cpu_port(cpu_dp, ds) {
140 @@ -836,6 +840,17 @@ static int dsa_switch_setup_tag_protocol(struct dsa_switch *ds)
145 + if (ds->ops->connect_tag_protocol) {
146 + err = ds->ops->connect_tag_protocol(ds, tag_ops->proto);
149 + "Unable to connect to tag protocol \"%s\": %pe\n",
150 + tag_ops->name, ERR_PTR(err));
158 @@ -1136,6 +1151,46 @@ static void dsa_tree_teardown(struct dsa_switch_tree *dst)
162 +static int dsa_tree_bind_tag_proto(struct dsa_switch_tree *dst,
163 + const struct dsa_device_ops *tag_ops)
165 + const struct dsa_device_ops *old_tag_ops = dst->tag_ops;
166 + struct dsa_notifier_tag_proto_info info;
169 + dst->tag_ops = tag_ops;
171 + /* Notify the new tagger about the connection to this tree */
172 + if (tag_ops->connect) {
173 + err = tag_ops->connect(dst);
178 + /* Notify the switches from this tree about the connection
179 + * to the new tagger
181 + info.tag_ops = tag_ops;
182 + err = dsa_tree_notify(dst, DSA_NOTIFIER_TAG_PROTO_CONNECT, &info);
183 + if (err && err != -EOPNOTSUPP)
184 + goto out_disconnect;
186 + /* Notify the old tagger about the disconnection from this tree */
187 + if (old_tag_ops->disconnect)
188 + old_tag_ops->disconnect(dst);
193 + /* Revert the new tagger's connection to this tree */
194 + if (tag_ops->disconnect)
195 + tag_ops->disconnect(dst);
197 + dst->tag_ops = old_tag_ops;
202 /* Since the dsa/tagging sysfs device attribute is per master, the assumption
203 * is that all DSA switches within a tree share the same tagger, otherwise
204 * they would have formed disjoint trees (different "dsa,member" values).
205 @@ -1168,12 +1223,15 @@ int dsa_tree_change_tag_proto(struct dsa_switch_tree *dst,
209 + /* Notify the tag protocol change */
210 info.tag_ops = tag_ops;
211 err = dsa_tree_notify(dst, DSA_NOTIFIER_TAG_PROTO, &info);
213 - goto out_unwind_tagger;
216 - dst->tag_ops = tag_ops;
217 + err = dsa_tree_bind_tag_proto(dst, tag_ops);
219 + goto out_unwind_tagger;
223 @@ -1260,6 +1318,7 @@ static int dsa_port_parse_cpu(struct dsa_port *dp, struct net_device *master,
224 struct dsa_switch_tree *dst = ds->dst;
225 const struct dsa_device_ops *tag_ops;
226 enum dsa_tag_protocol default_proto;
229 /* Find out which protocol the switch would prefer. */
230 default_proto = dsa_get_tag_protocol(dp, master);
231 @@ -1307,6 +1366,12 @@ static int dsa_port_parse_cpu(struct dsa_port *dp, struct net_device *master,
233 dsa_tag_driver_put(tag_ops);
235 + if (tag_ops->connect) {
236 + err = tag_ops->connect(dst);
241 dst->tag_ops = tag_ops;
244 diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
245 index 38ce5129a33dc..0db2b26b0c83c 100644
246 --- a/net/dsa/dsa_priv.h
247 +++ b/net/dsa/dsa_priv.h
248 @@ -37,6 +37,7 @@ enum {
249 DSA_NOTIFIER_VLAN_DEL,
251 DSA_NOTIFIER_TAG_PROTO,
252 + DSA_NOTIFIER_TAG_PROTO_CONNECT,
253 DSA_NOTIFIER_MRP_ADD,
254 DSA_NOTIFIER_MRP_DEL,
255 DSA_NOTIFIER_MRP_ADD_RING_ROLE,
256 diff --git a/net/dsa/switch.c b/net/dsa/switch.c
257 index 9c92edd969612..06948f5368296 100644
258 --- a/net/dsa/switch.c
259 +++ b/net/dsa/switch.c
260 @@ -647,6 +647,17 @@ static int dsa_switch_change_tag_proto(struct dsa_switch *ds,
264 +static int dsa_switch_connect_tag_proto(struct dsa_switch *ds,
265 + struct dsa_notifier_tag_proto_info *info)
267 + const struct dsa_device_ops *tag_ops = info->tag_ops;
269 + if (!ds->ops->connect_tag_protocol)
270 + return -EOPNOTSUPP;
272 + return ds->ops->connect_tag_protocol(ds, tag_ops->proto);
275 static int dsa_switch_mrp_add(struct dsa_switch *ds,
276 struct dsa_notifier_mrp_info *info)
278 @@ -766,6 +777,9 @@ static int dsa_switch_event(struct notifier_block *nb,
279 case DSA_NOTIFIER_TAG_PROTO:
280 err = dsa_switch_change_tag_proto(ds, info);
282 + case DSA_NOTIFIER_TAG_PROTO_CONNECT:
283 + err = dsa_switch_connect_tag_proto(ds, info);
285 case DSA_NOTIFIER_MRP_ADD:
286 err = dsa_switch_mrp_add(ds, info);