On Fri, 22 Jan 2021 14:36:08 +0100 Rasmus Villemoes wrote: > On 22/01/2021 10.05, Horatiu Vultur wrote: > > The 01/22/2021 00:43, Rasmus Villemoes wrote: > >> > >> It's not true that switchdev_port_obj_notify() only inspects the > >> ->handled field of "struct switchdev_notifier_port_obj_info" if > >> call_switchdev_blocking_notifiers() returns 0 - there's a WARN_ON() > >> triggering for a non-zero return combined with ->handled not being > >> true. But the real problem here is that -EOPNOTSUPP is not being > >> properly handled. > >> > >> The wrapper functions switchdev_handle_port_obj_add() et al change a > >> return value of -EOPNOTSUPP to 0, and the treatment of ->handled in > >> switchdev_port_obj_notify() seems to be designed to change that back > >> to -EOPNOTSUPP in case nobody actually acted on the notifier (i.e., > >> everybody returned -EOPNOTSUPP). > >> > >> Currently, as soon as some device down the stack passes the check_cb() > >> check, ->handled gets set to true, which means that > >> switchdev_port_obj_notify() cannot actually ever return -EOPNOTSUPP. > >> > >> This, for example, means that the detection of hardware offload > >> support in the MRP code is broken - br_mrp_set_ring_role() always ends > >> up setting mrp->ring_role_offloaded to 1, despite not a single > >> mainline driver implementing any of the SWITCHDEV_OBJ_ID*_MRP. So > >> since the MRP code thinks the generation of MRP test frames has been > >> offloaded, no such frames are actually put on the wire. > > > > Just a small correction to what you have said regarding MRP. Is not the > > option mrp->ring_role_offload that determines if the MRP Test frames are > > generated by the HW but is the return value of the function > > 'br_mrp_switchdev_send_ring_test' called from function > > 'br_mrp_start_test'. > > Hm, you're right, but the underlying problem is the same: > br_mrp_switchdev_set_ring_role() (whose return value is what is used to > determine ->ing_role_offloaded) and br_mrp_switchdev_send_ring_test() > both make use of switchdev_port_obj_add(SWITCHDEV_OBJ_ID_*MRP), and that > function is currently unable to return -EOPNOTSUPP, which it must in > order for the MRP logic to work.
Could you reword the commit message to avoid confusion for folks reading git history and repost? You can keep Petr's tag.
