| Message ID | 20210216214205.32385-3-horatiu.vultur@microchip.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | c513efa20c5254ef74c4157a03d515abdc46c503 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | bridge: mrp: Extend br_mrp_switchdev_* | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Clearly marked for net-next |
| netdev/subject_prefix | success | Link |
| netdev/cc_maintainers | success | CCed 5 of 5 maintainers |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 156 this patch: 156 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 14 lines checked |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 182 this patch: 182 |
| netdev/header_inline | success | Link |
| netdev/stable | success | Stable not CCed |
On Tue, Feb 16, 2021 at 10:41:59PM +0100, Horatiu Vultur wrote: > Add the member sw_backup to the structures switchdev_obj_ring_role_mrp > and switchdev_obj_in_role_mrp. In this way the SW can call the driver in > 2 ways, once when sw_backup is set to false, meaning that the driver > should implement this completely in HW. And if that is not supported the > SW will call again but with sw_backup set to true, meaning that the > HW should help or allow the SW to run the protocol. > > For example when role is MRM, if the HW can't detect when it stops > receiving MRP Test frames but it can trap these frames to CPU, then it > needs to return -EOPNOTSUPP when sw_backup is false and return 0 when > sw_backup is true. > > Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com> > --- > include/net/switchdev.h | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/include/net/switchdev.h b/include/net/switchdev.h > index 465362d9d063..b7fc7d0f54e2 100644 > --- a/include/net/switchdev.h > +++ b/include/net/switchdev.h > @@ -127,6 +127,7 @@ struct switchdev_obj_ring_role_mrp { > struct switchdev_obj obj; > u8 ring_role; > u32 ring_id; > + u8 sw_backup; > }; > > #define SWITCHDEV_OBJ_RING_ROLE_MRP(OBJ) \ > @@ -161,6 +162,7 @@ struct switchdev_obj_in_role_mrp { > u32 ring_id; > u16 in_id; > u8 in_role; > + u8 sw_backup; What was wrong with 'bool'? > }; > > #define SWITCHDEV_OBJ_IN_ROLE_MRP(OBJ) \ > -- > 2.27.0 > If a driver implements full MRP offload for a ring/interconnect manager/automanager, should it return -EOPNOTSUPP when sw_backup=false?
The 02/17/2021 10:34, Vladimir Oltean wrote: Hi Vladimir, > > On Tue, Feb 16, 2021 at 10:41:59PM +0100, Horatiu Vultur wrote: > > Add the member sw_backup to the structures switchdev_obj_ring_role_mrp > > and switchdev_obj_in_role_mrp. In this way the SW can call the driver in > > 2 ways, once when sw_backup is set to false, meaning that the driver > > should implement this completely in HW. And if that is not supported the > > SW will call again but with sw_backup set to true, meaning that the > > HW should help or allow the SW to run the protocol. > > > > For example when role is MRM, if the HW can't detect when it stops > > receiving MRP Test frames but it can trap these frames to CPU, then it > > needs to return -EOPNOTSUPP when sw_backup is false and return 0 when > > sw_backup is true. > > > > Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com> > > --- > > include/net/switchdev.h | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/include/net/switchdev.h b/include/net/switchdev.h > > index 465362d9d063..b7fc7d0f54e2 100644 > > --- a/include/net/switchdev.h > > +++ b/include/net/switchdev.h > > @@ -127,6 +127,7 @@ struct switchdev_obj_ring_role_mrp { > > struct switchdev_obj obj; > > u8 ring_role; > > u32 ring_id; > > + u8 sw_backup; > > }; > > > > #define SWITCHDEV_OBJ_RING_ROLE_MRP(OBJ) \ > > @@ -161,6 +162,7 @@ struct switchdev_obj_in_role_mrp { > > u32 ring_id; > > u16 in_id; > > u8 in_role; > > + u8 sw_backup; > > What was wrong with 'bool'? Yeah, that would be a better match. > > > }; > > > > #define SWITCHDEV_OBJ_IN_ROLE_MRP(OBJ) \ > > -- > > 2.27.0 > > > > If a driver implements full MRP offload for a ring/interconnect > manager/automanager, should it return -EOPNOTSUPP when sw_backup=false? In that case it should return 0. So if the driver can: - fully support MRP, when sw_backup = false, return 0. Then end of story. - partially support MRP, when sw_backup = false, return -EOPNOTSUPP, when sw_backup = true, return 0. - no support at all, return -EOPNOTSUPP.
On Wed, Feb 17, 2021 at 04:58:45PM +0100, Horatiu Vultur wrote: > > If a driver implements full MRP offload for a ring/interconnect > > manager/automanager, should it return -EOPNOTSUPP when sw_backup=false? > > In that case it should return 0. > So if the driver can: > - fully support MRP, when sw_backup = false, return 0. Then end of story. > - partially support MRP, when sw_backup = false, return -EOPNOTSUPP, > when sw_backup = true, return 0. > - no support at all, return -EOPNOTSUPP. Damn, I asked the wrong question. I meant to ask about what it should return when sw_backup=true. But you answered anyway that if it returns 0 when sw_backup=false, it can simply not deal with the case where sw_backup=true, because that is never supposed to happen.
diff --git a/include/net/switchdev.h b/include/net/switchdev.h index 465362d9d063..b7fc7d0f54e2 100644 --- a/include/net/switchdev.h +++ b/include/net/switchdev.h @@ -127,6 +127,7 @@ struct switchdev_obj_ring_role_mrp { struct switchdev_obj obj; u8 ring_role; u32 ring_id; + u8 sw_backup; }; #define SWITCHDEV_OBJ_RING_ROLE_MRP(OBJ) \ @@ -161,6 +162,7 @@ struct switchdev_obj_in_role_mrp { u32 ring_id; u16 in_id; u8 in_role; + u8 sw_backup; }; #define SWITCHDEV_OBJ_IN_ROLE_MRP(OBJ) \
Add the member sw_backup to the structures switchdev_obj_ring_role_mrp and switchdev_obj_in_role_mrp. In this way the SW can call the driver in 2 ways, once when sw_backup is set to false, meaning that the driver should implement this completely in HW. And if that is not supported the SW will call again but with sw_backup set to true, meaning that the HW should help or allow the SW to run the protocol. For example when role is MRM, if the HW can't detect when it stops receiving MRP Test frames but it can trap these frames to CPU, then it needs to return -EOPNOTSUPP when sw_backup is false and return 0 when sw_backup is true. Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com> --- include/net/switchdev.h | 2 ++ 1 file changed, 2 insertions(+)