History

net: switchdev: introduce a fanout helper for SWITCHDEV_FDB_{ADD,DEL}_TO_DEVICE

Currently DSA has an issue with FDB entries pointing towards the bridge in the presence of br_fdb_replay() being called at port join and leave time. In particular, each bridge port will ask for a replay for the FDB entries pointing towards the bridge when it joins, and for another replay when it leaves. This means that for example, a bridge with 4 switch ports will notify DSA 4 times of the bridge MAC address. But if the MAC address of the bridge changes during the normal runtime of the system, the bridge notifies switchdev [ once ] of the deletion of the old MAC address as a local FDB towards the bridge, and of the insertion [ again once ] of the new MAC address as a local FDB. This is a problem, because DSA keeps the old MAC address as a host FDB entry with refcount 4 (4 ports asked for it using br_fdb_replay). So the old MAC address will not be deleted. Additionally, the new MAC address will only be installed with refcount 1, and when the first switch port leaves the bridge (leaving 3 others as still members), it will delete with it the new MAC address of the bridge from the local FDB entries kept by DSA (because the br_fdb_replay call on deletion will bring the entry's refcount from 1 to 0). So the problem, really, is that the number of br_fdb_replay() calls is not matched with the refcount that a host FDB is offloaded to DSA during normal runtime. An elegant way to solve the problem would be to make the switchdev notification emitted by br_fdb_change_mac_address() result in a host FDB kept by DSA which has a refcount exactly equal to the number of ports under that bridge. Then, no matter how many DSA ports join or leave that bridge, the host FDB entry will always be deleted when there are exactly zero remaining DSA switch ports members of the bridge. To implement the proposed solution, we remember that the switchdev objects and port attributes have some helpers provided by switchdev, which can be optionally called by drivers: switchdev_handle_port_obj_{add,del} and switchdev_handle_port_attr_set. These helpers: - fan out a switchdev object/attribute emitted for the bridge towards all the lower interfaces that pass the check_cb(). - fan out a switchdev object/attribute emitted for a bridge port that is a LAG towards all the lower interfaces that pass the check_cb(). In other words, this is the model we need for the FDB events too: something that will keep an FDB entry emitted towards a physical port as it is, but translate an FDB entry emitted towards the bridge into N FDB entries, one per physical port. Of course, there are many differences between fanning out a switchdev object (VLAN) on 3 lower interfaces of a LAG and fanning out an FDB entry on 3 lower interfaces of a LAG. Intuitively, an FDB entry towards a LAG should be treated specially, because FDB entries are unicast, we can't just install the same address towards 3 destinations. It is imaginable that drivers might want to treat this case specifically, so create some methods for this case and do not recurse into the LAG lower ports, just the bridge ports. DSA also listens for FDB entries on "foreign" interfaces, aka interfaces bridged with us which are not part of our hardware domain: think an Ethernet switch bridged with a Wi-Fi AP. For those addresses, DSA installs host FDB entries. However, there we have the same problem (those host FDB entries are installed with a refcount of only 1) and an even bigger one which we did not have with FDB entries towards the bridge: br_fdb_replay() is currently not called for FDB entries on foreign interfaces, just for the physical port and for the bridge itself. So when DSA sniffs an address learned by the software bridge towards a foreign interface like an e1000 port, and then that e1000 leaves the bridge, DSA remains with the dangling host FDB address. That will be fixed separately by replaying all FDB entries and not just the ones towards the port and the bridge.

Vladimir Oltean committed 5 years ago

Commit 8ca07171 parent c6451cd2 files · +246 −0Browse files

Unified · Split

2 files
  1. include/net/switchdev.h +56 −0
  2. net/switchdev/switchdev.c +190 −0
include/net/switchdev.h+56−0
@@ -272,6 +272,30 @@ void switchdev_port_fwd_mark_set(struct net_device *dev, · 271 unmodified lines
272272 struct net_device *group_dev,
273273 bool joining);
274274
275+int switchdev_handle_fdb_add_to_device(struct net_device *dev,
276+ const struct switchdev_notifier_fdb_info *fdb_info,
277+ bool (*check_cb)(const struct net_device *dev),
278+ bool (*foreign_dev_check_cb)(const struct net_device *dev,
279+ const struct net_device *foreign_dev),
280+ int (*add_cb)(struct net_device *dev,
281+ const struct net_device *orig_dev, const void *ctx,
282+ const struct switchdev_notifier_fdb_info *fdb_info),
283+ int (*lag_add_cb)(struct net_device *dev,
284+ const struct net_device *orig_dev, const void *ctx,
285+ const struct switchdev_notifier_fdb_info *fdb_info));
286+
287+int switchdev_handle_fdb_del_to_device(struct net_device *dev,
288+ const struct switchdev_notifier_fdb_info *fdb_info,
289+ bool (*check_cb)(const struct net_device *dev),
290+ bool (*foreign_dev_check_cb)(const struct net_device *dev,
291+ const struct net_device *foreign_dev),
292+ int (*del_cb)(struct net_device *dev,
293+ const struct net_device *orig_dev, const void *ctx,
294+ const struct switchdev_notifier_fdb_info *fdb_info),
295+ int (*lag_del_cb)(struct net_device *dev,
296+ const struct net_device *orig_dev, const void *ctx,
297+ const struct switchdev_notifier_fdb_info *fdb_info));
298+
275299 int switchdev_handle_port_obj_add(struct net_device *dev,
276300 struct switchdev_notifier_port_obj_info *port_obj_info,
277301 bool (*check_cb)(const struct net_device *dev),
@@ -355,6 +379,38 @@ call_switchdev_blocking_notifiers(unsigned long val, · 77 unmodified lines
355379 return NOTIFY_DONE;
356380 }
357381
382+static inline int
383+switchdev_handle_fdb_add_to_device(struct net_device *dev,
384+ const struct switchdev_notifier_fdb_info *fdb_info,
385+ bool (*check_cb)(const struct net_device *dev),
386+ bool (*foreign_dev_check_cb)(const struct net_device *dev,
387+ const struct net_device *foreign_dev),
388+ int (*add_cb)(struct net_device *dev,
389+ const struct net_device *orig_dev, const void *ctx,
390+ const struct switchdev_notifier_fdb_info *fdb_info),
391+ int (*lag_add_cb)(struct net_device *dev,
392+ const struct net_device *orig_dev, const void *ctx,
393+ const struct switchdev_notifier_fdb_info *fdb_info))
394+{
395+ return 0;
396+}
397+
398+static inline int
399+switchdev_handle_fdb_del_to_device(struct net_device *dev,
400+ const struct switchdev_notifier_fdb_info *fdb_info,
401+ bool (*check_cb)(const struct net_device *dev),
402+ bool (*foreign_dev_check_cb)(const struct net_device *dev,
403+ const struct net_device *foreign_dev),
404+ int (*del_cb)(struct net_device *dev,
405+ const struct net_device *orig_dev, const void *ctx,
406+ const struct switchdev_notifier_fdb_info *fdb_info),
407+ int (*lag_del_cb)(struct net_device *dev,
408+ const struct net_device *orig_dev, const void *ctx,
409+ const struct switchdev_notifier_fdb_info *fdb_info));
410+{
411+ return 0;
412+}
413+
358414 static inline int
359415 switchdev_handle_port_obj_add(struct net_device *dev,
360416 struct switchdev_notifier_port_obj_info *port_obj_info,
net/switchdev/switchdev.c+190−0
@@ -378,6 +378,196 @@ int call_switchdev_blocking_notifiers(unsigned long val, struct net_device *dev, · 377 unmodified lines
378378 }
379379 EXPORT_SYMBOL_GPL(call_switchdev_blocking_notifiers);
380380
381+static int __switchdev_handle_fdb_add_to_device(struct net_device *dev,
382+ const struct net_device *orig_dev,
383+ const struct switchdev_notifier_fdb_info *fdb_info,
384+ bool (*check_cb)(const struct net_device *dev),
385+ bool (*foreign_dev_check_cb)(const struct net_device *dev,
386+ const struct net_device *foreign_dev),
387+ int (*add_cb)(struct net_device *dev,
388+ const struct net_device *orig_dev, const void *ctx,
389+ const struct switchdev_notifier_fdb_info *fdb_info),
390+ int (*lag_add_cb)(struct net_device *dev,
391+ const struct net_device *orig_dev, const void *ctx,
392+ const struct switchdev_notifier_fdb_info *fdb_info))
393+{
394+ const struct switchdev_notifier_info *info = &fdb_info->info;
395+ struct net_device *lower_dev;
396+ struct list_head *iter;
397+ int err = -EOPNOTSUPP;
398+
399+ if (check_cb(dev)) {
400+ /* Handle FDB entries on foreign interfaces as FDB entries
401+ * towards the software bridge.
402+ */
403+ if (foreign_dev_check_cb && foreign_dev_check_cb(dev, orig_dev)) {
404+ struct net_device *br = netdev_master_upper_dev_get_rcu(dev);
405+
406+ if (!br || !netif_is_bridge_master(br))
407+ return 0;
408+
409+ /* No point in handling FDB entries on a foreign bridge */
410+ if (foreign_dev_check_cb(dev, br))
411+ return 0;
412+
413+ return __switchdev_handle_fdb_add_to_device(br, orig_dev,
414+ fdb_info, check_cb,
415+ foreign_dev_check_cb,
416+ add_cb, lag_add_cb);
417+ }
418+
419+ return add_cb(dev, orig_dev, info->ctx, fdb_info);
420+ }
421+
422+ /* If we passed over the foreign check, it means that the LAG interface
423+ * is offloaded.
424+ */
425+ if (netif_is_lag_master(dev)) {
426+ if (!lag_add_cb)
427+ return -EOPNOTSUPP;
428+
429+ return lag_add_cb(dev, orig_dev, info->ctx, fdb_info);
430+ }
431+
432+ /* Recurse through lower interfaces in case the FDB entry is pointing
433+ * towards a bridge device.
434+ */
435+ netdev_for_each_lower_dev(dev, lower_dev, iter) {
436+ /* Do not propagate FDB entries across bridges */
437+ if (netif_is_bridge_master(lower_dev))
438+ continue;
439+
440+ err = __switchdev_handle_fdb_add_to_device(lower_dev, orig_dev,
441+ fdb_info, check_cb,
442+ foreign_dev_check_cb,
443+ add_cb, lag_add_cb);
444+ if (err && err != -EOPNOTSUPP)
445+ return err;
446+ }
447+
448+ return err;
449+}
450+
451+int switchdev_handle_fdb_add_to_device(struct net_device *dev,
452+ const struct switchdev_notifier_fdb_info *fdb_info,
453+ bool (*check_cb)(const struct net_device *dev),
454+ bool (*foreign_dev_check_cb)(const struct net_device *dev,
455+ const struct net_device *foreign_dev),
456+ int (*add_cb)(struct net_device *dev,
457+ const struct net_device *orig_dev, const void *ctx,
458+ const struct switchdev_notifier_fdb_info *fdb_info),
459+ int (*lag_add_cb)(struct net_device *dev,
460+ const struct net_device *orig_dev, const void *ctx,
461+ const struct switchdev_notifier_fdb_info *fdb_info))
462+{
463+ int err;
464+
465+ err = __switchdev_handle_fdb_add_to_device(dev, dev, fdb_info,
466+ check_cb,
467+ foreign_dev_check_cb,
468+ add_cb, lag_add_cb);
469+ if (err == -EOPNOTSUPP)
470+ err = 0;
471+
472+ return err;
473+}
474+EXPORT_SYMBOL_GPL(switchdev_handle_fdb_add_to_device);
475+
476+static int __switchdev_handle_fdb_del_to_device(struct net_device *dev,
477+ const struct net_device *orig_dev,
478+ const struct switchdev_notifier_fdb_info *fdb_info,
479+ bool (*check_cb)(const struct net_device *dev),
480+ bool (*foreign_dev_check_cb)(const struct net_device *dev,
481+ const struct net_device *foreign_dev),
482+ int (*del_cb)(struct net_device *dev,
483+ const struct net_device *orig_dev, const void *ctx,
484+ const struct switchdev_notifier_fdb_info *fdb_info),
485+ int (*lag_del_cb)(struct net_device *dev,
486+ const struct net_device *orig_dev, const void *ctx,
487+ const struct switchdev_notifier_fdb_info *fdb_info))
488+{
489+ const struct switchdev_notifier_info *info = &fdb_info->info;
490+ struct net_device *lower_dev;
491+ struct list_head *iter;
492+ int err = -EOPNOTSUPP;
493+
494+ if (check_cb(dev)) {
495+ /* Handle FDB entries on foreign interfaces as FDB entries
496+ * towards the software bridge.
497+ */
498+ if (foreign_dev_check_cb && foreign_dev_check_cb(dev, orig_dev)) {
499+ struct net_device *br = netdev_master_upper_dev_get_rcu(dev);
500+
501+ if (!br || !netif_is_bridge_master(br))
502+ return 0;
503+
504+ /* No point in handling FDB entries on a foreign bridge */
505+ if (foreign_dev_check_cb(dev, br))
506+ return 0;
507+
508+ return __switchdev_handle_fdb_del_to_device(br, orig_dev,
509+ fdb_info, check_cb,
510+ foreign_dev_check_cb,
511+ del_cb, lag_del_cb);
512+ }
513+
514+ return del_cb(dev, orig_dev, info->ctx, fdb_info);
515+ }
516+
517+ /* If we passed over the foreign check, it means that the LAG interface
518+ * is offloaded.
519+ */
520+ if (netif_is_lag_master(dev)) {
521+ if (!lag_del_cb)
522+ return -EOPNOTSUPP;
523+
524+ return lag_del_cb(dev, orig_dev, info->ctx, fdb_info);
525+ }
526+
527+ /* Recurse through lower interfaces in case the FDB entry is pointing
528+ * towards a bridge device.
529+ */
530+ netdev_for_each_lower_dev(dev, lower_dev, iter) {
531+ /* Do not propagate FDB entries across bridges */
532+ if (netif_is_bridge_master(lower_dev))
533+ continue;
534+
535+ err = switchdev_handle_fdb_del_to_device(lower_dev, fdb_info,
536+ check_cb,
537+ foreign_dev_check_cb,
538+ del_cb, lag_del_cb);
539+ if (err && err != -EOPNOTSUPP)
540+ return err;
541+ }
542+
543+ return err;
544+}
545+
546+int switchdev_handle_fdb_del_to_device(struct net_device *dev,
547+ const struct switchdev_notifier_fdb_info *fdb_info,
548+ bool (*check_cb)(const struct net_device *dev),
549+ bool (*foreign_dev_check_cb)(const struct net_device *dev,
550+ const struct net_device *foreign_dev),
551+ int (*del_cb)(struct net_device *dev,
552+ const struct net_device *orig_dev, const void *ctx,
553+ const struct switchdev_notifier_fdb_info *fdb_info),
554+ int (*lag_del_cb)(struct net_device *dev,
555+ const struct net_device *orig_dev, const void *ctx,
556+ const struct switchdev_notifier_fdb_info *fdb_info))
557+{
558+ int err;
559+
560+ err = __switchdev_handle_fdb_del_to_device(dev, dev, fdb_info,
561+ check_cb,
562+ foreign_dev_check_cb,
563+ del_cb, lag_del_cb);
564+ if (err == -EOPNOTSUPP)
565+ err = 0;
566+
567+ return err;
568+}
569+EXPORT_SYMBOL_GPL(switchdev_handle_fdb_del_to_device);
570+
381571 static int __switchdev_handle_port_obj_add(struct net_device *dev,
382572 struct switchdev_notifier_port_obj_info *port_obj_info,
383573 bool (*check_cb)(const struct net_device *dev),