| Message ID | 20210209202112.2545325-1-horatiu.vultur@microchip.com |
|---|---|
| Headers | show |
| Series | bridge: mrp: Extend br_mrp_switchdev_* | expand |
Hi Horatiu, On Tue, Feb 09, 2021 at 09:21:07PM +0100, Horatiu Vultur wrote: > This patch series extends MRP switchdev to allow the SW to have a better > understanding if the HW can implement the MRP functionality or it needs > to help the HW to run it. There are 3 cases: > - when HW can't implement at all the functionality. > - when HW can implement a part of the functionality but needs the SW > implement the rest. For example if it can't detect when it stops > receiving MRP Test frames but it can copy the MRP frames to CPU to > allow the SW to determine this. Another example is generating the MRP > Test frames. If HW can't do that then the SW is used as backup. > - when HW can implement completely the functionality. > > So, initially the SW tries to offload the entire functionality in HW, if > that fails it tries offload parts of the functionality in HW and use the > SW as helper and if also this fails then MRP can't run on this HW. > > Also implement the switchdev calls for Ocelot driver. This is an example > where the HW can't run completely the functionality but it can help the SW > to run it, by trapping all MRP frames to CPU. > > v3: > - implement the switchdev calls needed by Ocelot driver. > v2: > - fix typos in comments and in commit messages > - remove some of the comments > - move repeated code in helper function > - fix issue when deleting a node when sw_backup was true > > Horatiu Vultur (5): > switchdev: mrp: Extend ring_role_mrp and in_role_mrp > bridge: mrp: Add 'enum br_mrp_hw_support' > bridge: mrp: Extend br_mrp_switchdev to detect better the errors > bridge: mrp: Update br_mrp to use new return values of > br_mrp_switchdev > net: mscc: ocelot: Add support for MRP > > drivers/net/ethernet/mscc/ocelot_net.c | 154 +++++++++++++++++++ > drivers/net/ethernet/mscc/ocelot_vsc7514.c | 6 + > include/net/switchdev.h | 2 + > include/soc/mscc/ocelot.h | 6 + > net/bridge/br_mrp.c | 43 ++++-- > net/bridge/br_mrp_switchdev.c | 171 +++++++++++++-------- > net/bridge/br_private_mrp.h | 38 +++-- > 7 files changed, 327 insertions(+), 93 deletions(-) > > -- > 2.27.0 > Which net-next commit can these patches be applied to? On the current master I get: Applying: switchdev: mrp: Extend ring_role_mrp and in_role_mrp Applying: bridge: mrp: Add 'enum br_mrp_hw_support' Applying: bridge: mrp: Extend br_mrp_switchdev to detect better the errors error: patch failed: net/bridge/br_mrp_switchdev.c:177 error: net/bridge/br_mrp_switchdev.c: patch does not apply Patch failed at 0004 bridge: mrp: Extend br_mrp_switchdev to detect better the errors hint: Use 'git am --show-current-patch' to see the failed patch When you have resolved this problem, run "git am --continue". If you prefer to skip this patch, run "git am --skip" instead. To restore the original branch and stop patching, run "git am --abort".
Would you mind adding the switchdev MRP support for the DSA driver too, and move the code to the common ocelot library? I would like to give it a run. I think that's only fair, since I have to keep in sync the vsc7514 driver too for features that get added through DSA :) On Tue, Feb 09, 2021 at 09:21:12PM +0100, Horatiu Vultur wrote: > Add basic support for MRP. The HW will just trap all MRP frames on the > ring ports to CPU and allow the SW to process them. In this way it is > possible to for this node to behave both as MRM and MRC. > > Current limitations are: > - it doesn't support Interconnect roles. > - it supports only a single ring. > - the HW should be able to do forwarding of MRP Test frames so the SW > will not need to do this. So it would be able to have the role MRC > without SW support. > > Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com> > --- > drivers/net/ethernet/mscc/ocelot_net.c | 154 +++++++++++++++++++++ > drivers/net/ethernet/mscc/ocelot_vsc7514.c | 6 + > include/soc/mscc/ocelot.h | 6 + > 3 files changed, 166 insertions(+) > > diff --git a/drivers/net/ethernet/mscc/ocelot_net.c b/drivers/net/ethernet/mscc/ocelot_net.c > index 8f12fa45b1b5..65971403e823 100644 > --- a/drivers/net/ethernet/mscc/ocelot_net.c > +++ b/drivers/net/ethernet/mscc/ocelot_net.c > @@ -9,7 +9,10 @@ > */ > > #include <linux/if_bridge.h> > +#include <linux/mrp_bridge.h> > #include <net/pkt_cls.h> > +#include <soc/mscc/ocelot_vcap.h> > +#include <uapi/linux/mrp_bridge.h> > #include "ocelot.h" > #include "ocelot_vcap.h" > > @@ -1069,6 +1072,139 @@ static int ocelot_port_obj_del_mdb(struct net_device *dev, > return ocelot_port_mdb_del(ocelot, port, mdb); > } > > +#if IS_ENABLED(CONFIG_BRIDGE_MRP) > +static int ocelot_mrp_del_vcap(struct ocelot *ocelot, int port) > +{ > + struct ocelot_vcap_block *block_vcap_is2; > + struct ocelot_vcap_filter *filter; > + > + block_vcap_is2 = &ocelot->block[VCAP_IS2]; > + filter = ocelot_vcap_block_find_filter_by_id(block_vcap_is2, port, > + false); > + if (!filter) > + return 0; > + > + return ocelot_vcap_filter_del(ocelot, filter); > +} > + > +static int ocelot_add_mrp(struct net_device *dev, > + const struct switchdev_obj_mrp *mrp) > +{ > + struct ocelot_port_private *priv = netdev_priv(dev); > + struct ocelot_port *ocelot_port = &priv->port; > + struct ocelot *ocelot = ocelot_port->ocelot; > + > + if (mrp->p_port != dev && mrp->s_port != dev) > + return 0; > + > + if (ocelot->mrp_ring_id != 0 && > + ocelot->mrp_s_port && > + ocelot->mrp_p_port) > + return -EINVAL; > + > + if (mrp->p_port == dev) > + ocelot->mrp_p_port = dev; > + > + if (mrp->s_port == dev) > + ocelot->mrp_s_port = dev; > + > + ocelot->mrp_ring_id = mrp->ring_id; > + > + return 0; > +} > + > +static int ocelot_del_mrp(struct net_device *dev, > + const struct switchdev_obj_mrp *mrp) > +{ > + struct ocelot_port_private *priv = netdev_priv(dev); > + struct ocelot_port *ocelot_port = &priv->port; > + struct ocelot *ocelot = ocelot_port->ocelot; > + > + if (ocelot->mrp_p_port != dev && ocelot->mrp_s_port != dev) > + return 0; > + > + if (ocelot->mrp_ring_id == 0 && > + !ocelot->mrp_s_port && > + !ocelot->mrp_p_port) > + return -EINVAL; > + > + if (ocelot_mrp_del_vcap(ocelot, priv->chip_port)) > + return -EINVAL; > + > + if (ocelot->mrp_p_port == dev) > + ocelot->mrp_p_port = NULL; > + > + if (ocelot->mrp_s_port == dev) > + ocelot->mrp_s_port = NULL; > + > + ocelot->mrp_ring_id = 0; > + > + return 0; > +} > + > +static int ocelot_add_ring_role(struct net_device *dev, > + const struct switchdev_obj_ring_role_mrp *mrp) > +{ > + struct ocelot_port_private *priv = netdev_priv(dev); > + struct ocelot_port *ocelot_port = &priv->port; > + struct ocelot *ocelot = ocelot_port->ocelot; > + struct ocelot_vcap_filter *filter; > + int err; > + > + if (ocelot->mrp_ring_id != mrp->ring_id) > + return -EINVAL; > + > + if (!mrp->sw_backup) > + return -EOPNOTSUPP; > + > + if (ocelot->mrp_p_port != dev && ocelot->mrp_s_port != dev) > + return 0; > + > + filter = kzalloc(sizeof(*filter), GFP_KERNEL); > + if (!filter) > + return -ENOMEM; > + > + filter->key_type = OCELOT_VCAP_KEY_ETYPE; > + filter->prio = 1; > + filter->id.cookie = priv->chip_port; > + filter->id.tc_offload = false; > + filter->block_id = VCAP_IS2; > + filter->type = OCELOT_VCAP_FILTER_OFFLOAD; > + filter->ingress_port_mask = BIT(priv->chip_port); > + *(__be16 *)filter->key.etype.etype.value = htons(ETH_P_MRP); > + *(__be16 *)filter->key.etype.etype.mask = htons(0xffff); > + filter->action.mask_mode = OCELOT_MASK_MODE_PERMIT_DENY; > + filter->action.port_mask = 0x0; > + filter->action.cpu_copy_ena = true; > + filter->action.cpu_qu_num = 0; > + > + err = ocelot_vcap_filter_add(ocelot, filter, NULL); > + if (err) > + kfree(filter); > + > + return err; > +} > + > +static int ocelot_del_ring_role(struct net_device *dev, > + const struct switchdev_obj_ring_role_mrp *mrp) > +{ > + struct ocelot_port_private *priv = netdev_priv(dev); > + struct ocelot_port *ocelot_port = &priv->port; > + struct ocelot *ocelot = ocelot_port->ocelot; > + > + if (ocelot->mrp_ring_id != mrp->ring_id) > + return -EINVAL; > + > + if (!mrp->sw_backup) > + return -EOPNOTSUPP; > + > + if (ocelot->mrp_p_port != dev && ocelot->mrp_s_port != dev) > + return 0; > + > + return ocelot_mrp_del_vcap(ocelot, priv->chip_port); > +} > +#endif > + Would it make sense for this chunk of conditionally compiled code to stay in a separate file like ocelot_mrp.c? > static int ocelot_port_obj_add(struct net_device *dev, > const struct switchdev_obj *obj, > struct netlink_ext_ack *extack) > @@ -1083,6 +1219,15 @@ static int ocelot_port_obj_add(struct net_device *dev, > case SWITCHDEV_OBJ_ID_PORT_MDB: > ret = ocelot_port_obj_add_mdb(dev, SWITCHDEV_OBJ_PORT_MDB(obj)); > break; > +#if IS_ENABLED(CONFIG_BRIDGE_MRP) > + case SWITCHDEV_OBJ_ID_MRP: > + ret = ocelot_add_mrp(dev, SWITCHDEV_OBJ_MRP(obj)); > + break; > + case SWITCHDEV_OBJ_ID_RING_ROLE_MRP: > + ret = ocelot_add_ring_role(dev, > + SWITCHDEV_OBJ_RING_ROLE_MRP(obj)); > + break; > +#endif I'm not really sure why SWITCHDEV_OBJ_ID_MRP is conditionally defined. If you look at SWITCHDEV_ATTR_ID_BRIDGE_VLAN_FILTERING, that isn't conditionally defined, even though it depends on CONFIG_BRIDGE_VLAN_FILTERING at runtime. > default: > return -EOPNOTSUPP; > } > @@ -1103,6 +1248,15 @@ static int ocelot_port_obj_del(struct net_device *dev, > case SWITCHDEV_OBJ_ID_PORT_MDB: > ret = ocelot_port_obj_del_mdb(dev, SWITCHDEV_OBJ_PORT_MDB(obj)); > break; > +#if IS_ENABLED(CONFIG_BRIDGE_MRP) > + case SWITCHDEV_OBJ_ID_MRP: > + ret = ocelot_del_mrp(dev, SWITCHDEV_OBJ_MRP(obj)); > + break; > + case SWITCHDEV_OBJ_ID_RING_ROLE_MRP: > + ret = ocelot_del_ring_role(dev, > + SWITCHDEV_OBJ_RING_ROLE_MRP(obj)); > + break; > +#endif > default: > return -EOPNOTSUPP; > } > diff --git a/drivers/net/ethernet/mscc/ocelot_vsc7514.c b/drivers/net/ethernet/mscc/ocelot_vsc7514.c > index 6b6eb92149ba..96a9c9f98060 100644 > --- a/drivers/net/ethernet/mscc/ocelot_vsc7514.c > +++ b/drivers/net/ethernet/mscc/ocelot_vsc7514.c > @@ -698,6 +698,12 @@ static irqreturn_t ocelot_xtr_irq_handler(int irq, void *arg) > skb->offload_fwd_mark = 1; > > skb->protocol = eth_type_trans(skb, dev); > +#if IS_ENABLED(CONFIG_BRIDGE_MRP) > + if (skb->protocol == ntohs(ETH_P_MRP) && > + (priv->dev == ocelot->mrp_p_port || > + priv->dev == ocelot->mrp_s_port)) > + skb->offload_fwd_mark = 0; > +#endif I wonder if you could just reserve a certain CPUQ for trapped traffic, and just generically check for that, instead of MRP port roles? > if (!skb_defer_rx_timestamp(skb)) > netif_rx(skb); > dev->stats.rx_bytes += len; > diff --git a/include/soc/mscc/ocelot.h b/include/soc/mscc/ocelot.h > index d0d48e9620fb..d95c019ad84e 100644 > --- a/include/soc/mscc/ocelot.h > +++ b/include/soc/mscc/ocelot.h > @@ -682,6 +682,12 @@ struct ocelot { > /* Protects the PTP clock */ > spinlock_t ptp_clock_lock; > struct ptp_pin_desc ptp_pins[OCELOT_PTP_PINS_NUM]; > + > +#if IS_ENABLED(CONFIG_BRIDGE_MRP) > + u16 mrp_ring_id; > + struct net_device *mrp_p_port; > + struct net_device *mrp_s_port; > +#endif > }; > > struct ocelot_policer { > -- > 2.27.0 >
The 02/10/2021 10:08, Vladimir Oltean wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > Hi Horatiu, > > On Tue, Feb 09, 2021 at 09:21:07PM +0100, Horatiu Vultur wrote: > > This patch series extends MRP switchdev to allow the SW to have a better > > understanding if the HW can implement the MRP functionality or it needs > > to help the HW to run it. There are 3 cases: > > - when HW can't implement at all the functionality. > > - when HW can implement a part of the functionality but needs the SW > > implement the rest. For example if it can't detect when it stops > > receiving MRP Test frames but it can copy the MRP frames to CPU to > > allow the SW to determine this. Another example is generating the MRP > > Test frames. If HW can't do that then the SW is used as backup. > > - when HW can implement completely the functionality. > > > > So, initially the SW tries to offload the entire functionality in HW, if > > that fails it tries offload parts of the functionality in HW and use the > > SW as helper and if also this fails then MRP can't run on this HW. > > > > Also implement the switchdev calls for Ocelot driver. This is an example > > where the HW can't run completely the functionality but it can help the SW > > to run it, by trapping all MRP frames to CPU. > > > > v3: > > - implement the switchdev calls needed by Ocelot driver. > > v2: > > - fix typos in comments and in commit messages > > - remove some of the comments > > - move repeated code in helper function > > - fix issue when deleting a node when sw_backup was true > > > > Horatiu Vultur (5): > > switchdev: mrp: Extend ring_role_mrp and in_role_mrp > > bridge: mrp: Add 'enum br_mrp_hw_support' > > bridge: mrp: Extend br_mrp_switchdev to detect better the errors > > bridge: mrp: Update br_mrp to use new return values of > > br_mrp_switchdev > > net: mscc: ocelot: Add support for MRP > > > > drivers/net/ethernet/mscc/ocelot_net.c | 154 +++++++++++++++++++ > > drivers/net/ethernet/mscc/ocelot_vsc7514.c | 6 + > > include/net/switchdev.h | 2 + > > include/soc/mscc/ocelot.h | 6 + > > net/bridge/br_mrp.c | 43 ++++-- > > net/bridge/br_mrp_switchdev.c | 171 +++++++++++++-------- > > net/bridge/br_private_mrp.h | 38 +++-- > > 7 files changed, 327 insertions(+), 93 deletions(-) > > > > -- > > 2.27.0 > > > Hi Vladimir, > Which net-next commit can these patches be applied to? On the current > master I get: Sorry for this. I had an extra patch when I created these patches. And based on this I have added the patch series. This extra patch was this one: https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git/commit/?id=b2bdba1cbc84 Which was already applied to net. And I wanted to have it to be able to do more complete test of this patch series. Next time I should be more careful with this. > > Applying: switchdev: mrp: Extend ring_role_mrp and in_role_mrp > Applying: bridge: mrp: Add 'enum br_mrp_hw_support' > Applying: bridge: mrp: Extend br_mrp_switchdev to detect better the errors > error: patch failed: net/bridge/br_mrp_switchdev.c:177 > error: net/bridge/br_mrp_switchdev.c: patch does not apply > Patch failed at 0004 bridge: mrp: Extend br_mrp_switchdev to detect better the errors > hint: Use 'git am --show-current-patch' to see the failed patch > When you have resolved this problem, run "git am --continue". > If you prefer to skip this patch, run "git am --skip" instead. > To restore the original branch and stop patching, run "git am --abort". -- /Horatiu
The 02/10/2021 10:18, Vladimir Oltean wrote: Hi Vladimir, > > Would you mind adding the switchdev MRP support for the DSA driver too, > and move the code to the common ocelot library? I would like to give it > a run. I think that's only fair, since I have to keep in sync the > vsc7514 driver too for features that get added through DSA :) That is totally fair. I will do it in the next version :) > > On Tue, Feb 09, 2021 at 09:21:12PM +0100, Horatiu Vultur wrote: > > Add basic support for MRP. The HW will just trap all MRP frames on the > > ring ports to CPU and allow the SW to process them. In this way it is > > possible to for this node to behave both as MRM and MRC. > > > > Current limitations are: > > - it doesn't support Interconnect roles. > > - it supports only a single ring. > > - the HW should be able to do forwarding of MRP Test frames so the SW > > will not need to do this. So it would be able to have the role MRC > > without SW support. > > > > Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com> > > --- > > drivers/net/ethernet/mscc/ocelot_net.c | 154 +++++++++++++++++++++ > > drivers/net/ethernet/mscc/ocelot_vsc7514.c | 6 + > > include/soc/mscc/ocelot.h | 6 + > > 3 files changed, 166 insertions(+) > > > > diff --git a/drivers/net/ethernet/mscc/ocelot_net.c b/drivers/net/ethernet/mscc/ocelot_net.c > > index 8f12fa45b1b5..65971403e823 100644 > > --- a/drivers/net/ethernet/mscc/ocelot_net.c > > +++ b/drivers/net/ethernet/mscc/ocelot_net.c > > @@ -9,7 +9,10 @@ > > */ > > > > #include <linux/if_bridge.h> > > +#include <linux/mrp_bridge.h> > > #include <net/pkt_cls.h> > > +#include <soc/mscc/ocelot_vcap.h> > > +#include <uapi/linux/mrp_bridge.h> > > #include "ocelot.h" > > #include "ocelot_vcap.h" > > > > @@ -1069,6 +1072,139 @@ static int ocelot_port_obj_del_mdb(struct net_device *dev, > > return ocelot_port_mdb_del(ocelot, port, mdb); > > } > > > > +#if IS_ENABLED(CONFIG_BRIDGE_MRP) > > +static int ocelot_mrp_del_vcap(struct ocelot *ocelot, int port) > > +{ > > + struct ocelot_vcap_block *block_vcap_is2; > > + struct ocelot_vcap_filter *filter; > > + > > + block_vcap_is2 = &ocelot->block[VCAP_IS2]; > > + filter = ocelot_vcap_block_find_filter_by_id(block_vcap_is2, port, > > + false); > > + if (!filter) > > + return 0; > > + > > + return ocelot_vcap_filter_del(ocelot, filter); > > +} > > + > > +static int ocelot_add_mrp(struct net_device *dev, > > + const struct switchdev_obj_mrp *mrp) > > +{ > > + struct ocelot_port_private *priv = netdev_priv(dev); > > + struct ocelot_port *ocelot_port = &priv->port; > > + struct ocelot *ocelot = ocelot_port->ocelot; > > + > > + if (mrp->p_port != dev && mrp->s_port != dev) > > + return 0; > > + > > + if (ocelot->mrp_ring_id != 0 && > > + ocelot->mrp_s_port && > > + ocelot->mrp_p_port) > > + return -EINVAL; > > + > > + if (mrp->p_port == dev) > > + ocelot->mrp_p_port = dev; > > + > > + if (mrp->s_port == dev) > > + ocelot->mrp_s_port = dev; > > + > > + ocelot->mrp_ring_id = mrp->ring_id; > > + > > + return 0; > > +} > > + > > +static int ocelot_del_mrp(struct net_device *dev, > > + const struct switchdev_obj_mrp *mrp) > > +{ > > + struct ocelot_port_private *priv = netdev_priv(dev); > > + struct ocelot_port *ocelot_port = &priv->port; > > + struct ocelot *ocelot = ocelot_port->ocelot; > > + > > + if (ocelot->mrp_p_port != dev && ocelot->mrp_s_port != dev) > > + return 0; > > + > > + if (ocelot->mrp_ring_id == 0 && > > + !ocelot->mrp_s_port && > > + !ocelot->mrp_p_port) > > + return -EINVAL; > > + > > + if (ocelot_mrp_del_vcap(ocelot, priv->chip_port)) > > + return -EINVAL; > > + > > + if (ocelot->mrp_p_port == dev) > > + ocelot->mrp_p_port = NULL; > > + > > + if (ocelot->mrp_s_port == dev) > > + ocelot->mrp_s_port = NULL; > > + > > + ocelot->mrp_ring_id = 0; > > + > > + return 0; > > +} > > + > > +static int ocelot_add_ring_role(struct net_device *dev, > > + const struct switchdev_obj_ring_role_mrp *mrp) > > +{ > > + struct ocelot_port_private *priv = netdev_priv(dev); > > + struct ocelot_port *ocelot_port = &priv->port; > > + struct ocelot *ocelot = ocelot_port->ocelot; > > + struct ocelot_vcap_filter *filter; > > + int err; > > + > > + if (ocelot->mrp_ring_id != mrp->ring_id) > > + return -EINVAL; > > + > > + if (!mrp->sw_backup) > > + return -EOPNOTSUPP; > > + > > + if (ocelot->mrp_p_port != dev && ocelot->mrp_s_port != dev) > > + return 0; > > + > > + filter = kzalloc(sizeof(*filter), GFP_KERNEL); > > + if (!filter) > > + return -ENOMEM; > > + > > + filter->key_type = OCELOT_VCAP_KEY_ETYPE; > > + filter->prio = 1; > > + filter->id.cookie = priv->chip_port; > > + filter->id.tc_offload = false; > > + filter->block_id = VCAP_IS2; > > + filter->type = OCELOT_VCAP_FILTER_OFFLOAD; > > + filter->ingress_port_mask = BIT(priv->chip_port); > > + *(__be16 *)filter->key.etype.etype.value = htons(ETH_P_MRP); > > + *(__be16 *)filter->key.etype.etype.mask = htons(0xffff); > > + filter->action.mask_mode = OCELOT_MASK_MODE_PERMIT_DENY; > > + filter->action.port_mask = 0x0; > > + filter->action.cpu_copy_ena = true; > > + filter->action.cpu_qu_num = 0; > > + > > + err = ocelot_vcap_filter_add(ocelot, filter, NULL); > > + if (err) > > + kfree(filter); > > + > > + return err; > > +} > > + > > +static int ocelot_del_ring_role(struct net_device *dev, > > + const struct switchdev_obj_ring_role_mrp *mrp) > > +{ > > + struct ocelot_port_private *priv = netdev_priv(dev); > > + struct ocelot_port *ocelot_port = &priv->port; > > + struct ocelot *ocelot = ocelot_port->ocelot; > > + > > + if (ocelot->mrp_ring_id != mrp->ring_id) > > + return -EINVAL; > > + > > + if (!mrp->sw_backup) > > + return -EOPNOTSUPP; > > + > > + if (ocelot->mrp_p_port != dev && ocelot->mrp_s_port != dev) > > + return 0; > > + > > + return ocelot_mrp_del_vcap(ocelot, priv->chip_port); > > +} > > +#endif > > + > > Would it make sense for this chunk of conditionally compiled code to > stay in a separate file like ocelot_mrp.c? Also initially I was thinking to add this code in a different file but I was not sure. But I will do that in the next version. > > > static int ocelot_port_obj_add(struct net_device *dev, > > const struct switchdev_obj *obj, > > struct netlink_ext_ack *extack) > > @@ -1083,6 +1219,15 @@ static int ocelot_port_obj_add(struct net_device *dev, > > case SWITCHDEV_OBJ_ID_PORT_MDB: > > ret = ocelot_port_obj_add_mdb(dev, SWITCHDEV_OBJ_PORT_MDB(obj)); > > break; > > +#if IS_ENABLED(CONFIG_BRIDGE_MRP) > > + case SWITCHDEV_OBJ_ID_MRP: > > + ret = ocelot_add_mrp(dev, SWITCHDEV_OBJ_MRP(obj)); > > + break; > > + case SWITCHDEV_OBJ_ID_RING_ROLE_MRP: > > + ret = ocelot_add_ring_role(dev, > > + SWITCHDEV_OBJ_RING_ROLE_MRP(obj)); > > + break; > > +#endif > > I'm not really sure why SWITCHDEV_OBJ_ID_MRP is conditionally defined. > If you look at SWITCHDEV_ATTR_ID_BRIDGE_VLAN_FILTERING, that isn't > conditionally defined, even though it depends on CONFIG_BRIDGE_VLAN_FILTERING > at runtime. Then should I just create another patch for removing this? To be honest I also prefer that SWITCHDEV_OBJ_ID_MRP will not be conditionally defined. > > > default: > > return -EOPNOTSUPP; > > } > > @@ -1103,6 +1248,15 @@ static int ocelot_port_obj_del(struct net_device *dev, > > case SWITCHDEV_OBJ_ID_PORT_MDB: > > ret = ocelot_port_obj_del_mdb(dev, SWITCHDEV_OBJ_PORT_MDB(obj)); > > break; > > +#if IS_ENABLED(CONFIG_BRIDGE_MRP) > > + case SWITCHDEV_OBJ_ID_MRP: > > + ret = ocelot_del_mrp(dev, SWITCHDEV_OBJ_MRP(obj)); > > + break; > > + case SWITCHDEV_OBJ_ID_RING_ROLE_MRP: > > + ret = ocelot_del_ring_role(dev, > > + SWITCHDEV_OBJ_RING_ROLE_MRP(obj)); > > + break; > > +#endif > > default: > > return -EOPNOTSUPP; > > } > > diff --git a/drivers/net/ethernet/mscc/ocelot_vsc7514.c b/drivers/net/ethernet/mscc/ocelot_vsc7514.c > > index 6b6eb92149ba..96a9c9f98060 100644 > > --- a/drivers/net/ethernet/mscc/ocelot_vsc7514.c > > +++ b/drivers/net/ethernet/mscc/ocelot_vsc7514.c > > @@ -698,6 +698,12 @@ static irqreturn_t ocelot_xtr_irq_handler(int irq, void *arg) > > skb->offload_fwd_mark = 1; > > > > skb->protocol = eth_type_trans(skb, dev); > > +#if IS_ENABLED(CONFIG_BRIDGE_MRP) > > + if (skb->protocol == ntohs(ETH_P_MRP) && > > + (priv->dev == ocelot->mrp_p_port || > > + priv->dev == ocelot->mrp_s_port)) > > + skb->offload_fwd_mark = 0; > > +#endif > > I wonder if you could just reserve a certain CPUQ for trapped traffic, > and just generically check for that, instead of MRP port roles? That is a good idea, I would try to have a look. > > > if (!skb_defer_rx_timestamp(skb)) > > netif_rx(skb); > > dev->stats.rx_bytes += len; > > diff --git a/include/soc/mscc/ocelot.h b/include/soc/mscc/ocelot.h > > index d0d48e9620fb..d95c019ad84e 100644 > > --- a/include/soc/mscc/ocelot.h > > +++ b/include/soc/mscc/ocelot.h > > @@ -682,6 +682,12 @@ struct ocelot { > > /* Protects the PTP clock */ > > spinlock_t ptp_clock_lock; > > struct ptp_pin_desc ptp_pins[OCELOT_PTP_PINS_NUM]; > > + > > +#if IS_ENABLED(CONFIG_BRIDGE_MRP) > > + u16 mrp_ring_id; > > + struct net_device *mrp_p_port; > > + struct net_device *mrp_s_port; > > +#endif > > }; > > > > struct ocelot_policer { > > -- > > 2.27.0 > > -- /Horatiu
This patch series extends MRP switchdev to allow the SW to have a better understanding if the HW can implement the MRP functionality or it needs to help the HW to run it. There are 3 cases: - when HW can't implement at all the functionality. - when HW can implement a part of the functionality but needs the SW implement the rest. For example if it can't detect when it stops receiving MRP Test frames but it can copy the MRP frames to CPU to allow the SW to determine this. Another example is generating the MRP Test frames. If HW can't do that then the SW is used as backup. - when HW can implement completely the functionality. So, initially the SW tries to offload the entire functionality in HW, if that fails it tries offload parts of the functionality in HW and use the SW as helper and if also this fails then MRP can't run on this HW. Also implement the switchdev calls for Ocelot driver. This is an example where the HW can't run completely the functionality but it can help the SW to run it, by trapping all MRP frames to CPU. v3: - implement the switchdev calls needed by Ocelot driver. v2: - fix typos in comments and in commit messages - remove some of the comments - move repeated code in helper function - fix issue when deleting a node when sw_backup was true Horatiu Vultur (5): switchdev: mrp: Extend ring_role_mrp and in_role_mrp bridge: mrp: Add 'enum br_mrp_hw_support' bridge: mrp: Extend br_mrp_switchdev to detect better the errors bridge: mrp: Update br_mrp to use new return values of br_mrp_switchdev net: mscc: ocelot: Add support for MRP drivers/net/ethernet/mscc/ocelot_net.c | 154 +++++++++++++++++++ drivers/net/ethernet/mscc/ocelot_vsc7514.c | 6 + include/net/switchdev.h | 2 + include/soc/mscc/ocelot.h | 6 + net/bridge/br_mrp.c | 43 ++++-- net/bridge/br_mrp_switchdev.c | 171 +++++++++++++-------- net/bridge/br_private_mrp.h | 38 +++-- 7 files changed, 327 insertions(+), 93 deletions(-)