target/linux/generic/backport-5.15/701-v5.17-dsa-make-tagging-protocols-connect-to-individual-switches.patch

   1 From 7f2973149c22e7a6fee4c0c9fa6b8e4108e9c208 Mon Sep 17 00:00:00 2001
   2 From: Vladimir Oltean <vladimir.oltean@nxp.com>
   3 Date: Tue, 14 Dec 2021 03:45:36 +0200
   4 Subject: net: dsa: make tagging protocols connect to individual switches from
   5  a tree
   6
   7 On the NXP Bluebox 3 board which uses a multi-switch setup with sja1105,
   8 the mechanism through which the tagger connects to the switch tree is
   9 broken, due to improper DSA code design. At the time when tag_ops->connect()
  10 is called in dsa_port_parse_cpu(), DSA hasn't finished "touching" all
  11 the ports, so it doesn't know how large the tree is and how many ports
  12 it has. It has just seen the first CPU port by this time. As a result,
  13 this function will call the tagger's ->connect method too early, and the
  14 tagger will connect only to the first switch from the tree.
  15
  16 This could be perhaps addressed a bit more simply by just moving the
  17 tag_ops->connect(dst) call a bit later (for example in dsa_tree_setup),
  18 but there is already a design inconsistency at present: on the switch
  19 side, the notification is on a per-switch basis, but on the tagger side,
  20 it is on a per-tree basis. Furthermore, the persistent storage itself is
  21 per switch (ds->tagger_data). And the tagger connect and disconnect
  22 procedures (at least the ones that exist currently) could see a fair bit
  23 of simplification if they didn't have to iterate through the switches of
  24 a tree.
  25
  26 To fix the issue, this change transforms tag_ops->connect(dst) into
  27 tag_ops->connect(ds) and moves it somewhere where we already iterate
  28 over all switches of a tree. That is in dsa_switch_setup_tag_protocol(),
  29 which is a good placement because we already have there the connection
  30 call to the switch side of things.
  31
  32 As for the dsa_tree_bind_tag_proto() method (called from the code path
  33 that changes the tag protocol), things are a bit more complicated
  34 because we receive the tree as argument, yet when we unwind on errors,
  35 it would be nice to not call tag_ops->disconnect(ds) where we didn't
  36 previously call tag_ops->connect(ds). We didn't have this problem before
  37 because the tag_ops connection operations passed the entire dst before,
  38 and this is more fine grained now. To solve the error rewind case using
  39 the new API, we have to create yet one more cross-chip notifier for
  40 disconnection, and stay connected with the old tag protocol to all the
  41 switches in the tree until we've succeeded to connect with the new one
  42 as well. So if something fails half way, the whole tree is still
  43 connected to the old tagger. But there may still be leaks if the tagger
  44 fails to connect to the 2nd out of 3 switches in a tree: somebody needs
  45 to tell the tagger to disconnect from the first switch. Nothing comes
  46 for free, and this was previously handled privately by the tagging
  47 protocol driver before, but now we need to emit a disconnect cross-chip
  48 notifier for that, because DSA has to take care of the unwind path. We
  49 assume that the tagging protocol has connected to a switch if it has set
  50 ds->tagger_data to something, otherwise we avoid calling its
  51 disconnection method in the error rewind path.
  52
  53 The rest of the changes are in the tagging protocol drivers, and have to
  54 do with the replacement of dst with ds. The iteration is removed and the
  55 error unwind path is simplified, as mentioned above.
  56
  57 Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
  58 Signed-off-by: David S. Miller <davem@davemloft.net>
  59 ---
  60  include/net/dsa.h          |  5 ++--
  61  net/dsa/dsa2.c             | 44 +++++++++++++-----------------
  62  net/dsa/dsa_priv.h         |  1 +
  63  net/dsa/switch.c           | 52 ++++++++++++++++++++++++++++++++---
  64  net/dsa/tag_ocelot_8021q.c | 53 +++++++++++-------------------------
  65  net/dsa/tag_sja1105.c      | 67 ++++++++++++++++------------------------------
  66  6 files changed, 109 insertions(+), 113 deletions(-)
  67
  68 --- a/include/net/dsa.h
  69 +++ b/include/net/dsa.h
  70 @@ -80,15 +80,14 @@ enum dsa_tag_protocol {
  71  };
  72
  73  struct dsa_switch;
  74 -struct dsa_switch_tree;
  75
  76  struct dsa_device_ops {
  77         struct sk_buff *(*xmit)(struct sk_buff *skb, struct net_device *dev);
  78         struct sk_buff *(*rcv)(struct sk_buff *skb, struct net_device *dev);
  79         void (*flow_dissect)(const struct sk_buff *skb, __be16 *proto,
  80                              int *offset);
  81 -       int (*connect)(struct dsa_switch_tree *dst);
  82 -       void (*disconnect)(struct dsa_switch_tree *dst);
  83 +       int (*connect)(struct dsa_switch *ds);
  84 +       void (*disconnect)(struct dsa_switch *ds);
  85         unsigned int needed_headroom;
  86         unsigned int needed_tailroom;
  87         const char *name;
  88 --- a/net/dsa/dsa2.c
  89 +++ b/net/dsa/dsa2.c
  90 @@ -230,12 +230,8 @@ static struct dsa_switch_tree *dsa_tree_
  91
  92  static void dsa_tree_free(struct dsa_switch_tree *dst)
  93  {
  94 -       if (dst->tag_ops) {
  95 -               if (dst->tag_ops->disconnect)
  96 -                       dst->tag_ops->disconnect(dst);
  97 -
  98 +       if (dst->tag_ops)
  99                 dsa_tag_driver_put(dst->tag_ops);
 100 -       }
 101         list_del(&dst->list);
 102         kfree(dst);
 103  }
 104 @@ -826,17 +822,29 @@ static int dsa_switch_setup_tag_protocol
 105         }
 106
 107  connect:
 108 +       if (tag_ops->connect) {
 109 +               err = tag_ops->connect(ds);
 110 +               if (err)
 111 +                       return err;
 112 +       }
 113 +
 114         if (ds->ops->connect_tag_protocol) {
 115                 err = ds->ops->connect_tag_protocol(ds, tag_ops->proto);
 116                 if (err) {
 117                         dev_err(ds->dev,
 118                                 "Unable to connect to tag protocol \"%s\": %pe\n",
 119                                 tag_ops->name, ERR_PTR(err));
 120 -                       return err;
 121 +                       goto disconnect;
 122                 }
 123         }
 124
 125         return 0;
 126 +
 127 +disconnect:
 128 +       if (tag_ops->disconnect)
 129 +               tag_ops->disconnect(ds);
 130 +
 131 +       return err;
 132  }
 133
 134  static int dsa_switch_setup(struct dsa_switch *ds)
 135 @@ -1156,13 +1164,6 @@ static int dsa_tree_bind_tag_proto(struc
 136
 137         dst->tag_ops = tag_ops;
 138
 139 -       /* Notify the new tagger about the connection to this tree */
 140 -       if (tag_ops->connect) {
 141 -               err = tag_ops->connect(dst);
 142 -               if (err)
 143 -                       goto out_revert;
 144 -       }
 145 -
 146         /* Notify the switches from this tree about the connection
 147          * to the new tagger
 148          */
 149 @@ -1172,16 +1173,14 @@ static int dsa_tree_bind_tag_proto(struc
 150                 goto out_disconnect;
 151
 152         /* Notify the old tagger about the disconnection from this tree */
 153 -       if (old_tag_ops->disconnect)
 154 -               old_tag_ops->disconnect(dst);
 155 +       info.tag_ops = old_tag_ops;
 156 +       dsa_tree_notify(dst, DSA_NOTIFIER_TAG_PROTO_DISCONNECT, &info);
 157
 158         return 0;
 159
 160  out_disconnect:
 161 -       /* Revert the new tagger's connection to this tree */
 162 -       if (tag_ops->disconnect)
 163 -               tag_ops->disconnect(dst);
 164 -out_revert:
 165 +       info.tag_ops = tag_ops;
 166 +       dsa_tree_notify(dst, DSA_NOTIFIER_TAG_PROTO_DISCONNECT, &info);
 167         dst->tag_ops = old_tag_ops;
 168
 169         return err;
 170 @@ -1315,7 +1314,6 @@ static int dsa_port_parse_cpu(struct dsa
 171         struct dsa_switch *ds = dp->ds;
 172         struct dsa_switch_tree *dst = ds->dst;
 173         enum dsa_tag_protocol default_proto;
 174 -       int err;
 175
 176         /* Find out which protocol the switch would prefer. */
 177         default_proto = dsa_get_tag_protocol(dp, master);
 178 @@ -1370,12 +1368,6 @@ static int dsa_port_parse_cpu(struct dsa
 179                  */
 180                 dsa_tag_driver_put(tag_ops);
 181         } else {
 182 -               if (tag_ops->connect) {
 183 -                       err = tag_ops->connect(dst);
 184 -                       if (err)
 185 -                               return err;
 186 -               }
 187 -
 188                 dst->tag_ops = tag_ops;
 189         }
 190
 191 --- a/net/dsa/dsa_priv.h
 192 +++ b/net/dsa/dsa_priv.h
 193 @@ -38,6 +38,7 @@ enum {
 194         DSA_NOTIFIER_MTU,
 195         DSA_NOTIFIER_TAG_PROTO,
 196         DSA_NOTIFIER_TAG_PROTO_CONNECT,
 197 +       DSA_NOTIFIER_TAG_PROTO_DISCONNECT,
 198         DSA_NOTIFIER_MRP_ADD,
 199         DSA_NOTIFIER_MRP_DEL,
 200         DSA_NOTIFIER_MRP_ADD_RING_ROLE,
 201 --- a/net/dsa/switch.c
 202 +++ b/net/dsa/switch.c
 203 @@ -616,15 +616,58 @@ static int dsa_switch_change_tag_proto(s
 204         return 0;
 205  }
 206
 207 -static int dsa_switch_connect_tag_proto(struct dsa_switch *ds,
 208 -                                       struct dsa_notifier_tag_proto_info *info)
 209 +/* We use the same cross-chip notifiers to inform both the tagger side, as well
 210 + * as the switch side, of connection and disconnection events.
 211 + * Since ds->tagger_data is owned by the tagger, it isn't a hard error if the
 212 + * switch side doesn't support connecting to this tagger, and therefore, the
 213 + * fact that we don't disconnect the tagger side doesn't constitute a memory
 214 + * leak: the tagger will still operate with persistent per-switch memory, just
 215 + * with the switch side unconnected to it. What does constitute a hard error is
 216 + * when the switch side supports connecting but fails.
 217 + */
 218 +static int
 219 +dsa_switch_connect_tag_proto(struct dsa_switch *ds,
 220 +                            struct dsa_notifier_tag_proto_info *info)
 221  {
 222         const struct dsa_device_ops *tag_ops = info->tag_ops;
 223 +       int err;
 224 +
 225 +       /* Notify the new tagger about the connection to this switch */
 226 +       if (tag_ops->connect) {
 227 +               err = tag_ops->connect(ds);
 228 +               if (err)
 229 +                       return err;
 230 +       }
 231
 232         if (!ds->ops->connect_tag_protocol)
 233                 return -EOPNOTSUPP;
 234
 235 -       return ds->ops->connect_tag_protocol(ds, tag_ops->proto);
 236 +       /* Notify the switch about the connection to the new tagger */
 237 +       err = ds->ops->connect_tag_protocol(ds, tag_ops->proto);
 238 +       if (err) {
 239 +               /* Revert the new tagger's connection to this tree */
 240 +               if (tag_ops->disconnect)
 241 +                       tag_ops->disconnect(ds);
 242 +               return err;
 243 +       }
 244 +
 245 +       return 0;
 246 +}
 247 +
 248 +static int
 249 +dsa_switch_disconnect_tag_proto(struct dsa_switch *ds,
 250 +                               struct dsa_notifier_tag_proto_info *info)
 251 +{
 252 +       const struct dsa_device_ops *tag_ops = info->tag_ops;
 253 +
 254 +       /* Notify the tagger about the disconnection from this switch */
 255 +       if (tag_ops->disconnect && ds->tagger_data)
 256 +               tag_ops->disconnect(ds);
 257 +
 258 +       /* No need to notify the switch, since it shouldn't have any
 259 +        * resources to tear down
 260 +        */
 261 +       return 0;
 262  }
 263
 264  static int dsa_switch_mrp_add(struct dsa_switch *ds,
 265 @@ -749,6 +792,9 @@ static int dsa_switch_event(struct notif
 266         case DSA_NOTIFIER_TAG_PROTO_CONNECT:
 267                 err = dsa_switch_connect_tag_proto(ds, info);
 268                 break;
 269 +       case DSA_NOTIFIER_TAG_PROTO_DISCONNECT:
 270 +               err = dsa_switch_disconnect_tag_proto(ds, info);
 271 +               break;
 272         case DSA_NOTIFIER_MRP_ADD:
 273                 err = dsa_switch_mrp_add(ds, info);
 274                 break;