| Message ID | 20190812104827.5935-4-yangbo.lu@nxp.com |
|---|---|
| State | Changes Requested |
| Delegated to: | David Miller |
| Headers | show |
| Series | ocelot_ace: fix and improve the driver | expand |
The 08/12/2019 18:48, Yangbo Lu wrote: > The trap action should be copying the frame to CPU and > dropping it for forwarding, but current setting was just > copying frame to CPU. Are there any actions which do a "copy-to-cpu" and still forward the frame in HW? > Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com> > --- > drivers/net/ethernet/mscc/ocelot_ace.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/net/ethernet/mscc/ocelot_ace.c b/drivers/net/ethernet/mscc/ocelot_ace.c > index 91250f3..59ad590 100644 > --- a/drivers/net/ethernet/mscc/ocelot_ace.c > +++ b/drivers/net/ethernet/mscc/ocelot_ace.c > @@ -317,9 +317,9 @@ static void is2_action_set(struct vcap_data *data, > break; > case OCELOT_ACL_ACTION_TRAP: > VCAP_ACT_SET(PORT_MASK, 0x0); > - VCAP_ACT_SET(MASK_MODE, 0x0); > - VCAP_ACT_SET(POLICE_ENA, 0x0); > - VCAP_ACT_SET(POLICE_IDX, 0x0); > + VCAP_ACT_SET(MASK_MODE, 0x1); > + VCAP_ACT_SET(POLICE_ENA, 0x1); > + VCAP_ACT_SET(POLICE_IDX, OCELOT_POLICER_DISCARD); This seems wrong. The policer is used to ensure that traffic are discarded, even in the case where other users of the code has requested it to go to the CPU. Are you sure this is working? If it is working, then I fear we have an issue with the DROP action which uses this to discard frames. > VCAP_ACT_SET(CPU_QU_NUM, 0x0); > VCAP_ACT_SET(CPU_COPY_ENA, 0x1); > break; > -- > 2.7.4
Hi Allan, > -----Original Message----- > From: Allan W. Nielsen <allan.nielsen@microchip.com> > Sent: Monday, August 12, 2019 8:32 PM > To: Y.b. Lu <yangbo.lu@nxp.com> > Cc: netdev@vger.kernel.org; David S . Miller <davem@davemloft.net>; > Alexandre Belloni <alexandre.belloni@bootlin.com>; Microchip Linux Driver > Support <UNGLinuxDriver@microchip.com> > Subject: Re: [PATCH 3/3] ocelot_ace: fix action of trap > > The 08/12/2019 18:48, Yangbo Lu wrote: > > The trap action should be copying the frame to CPU and dropping it for > > forwarding, but current setting was just copying frame to CPU. > > Are there any actions which do a "copy-to-cpu" and still forward the frame in > HW? [Y.b. Lu] We're using Felix switch whose code hadn't been accepted by upstream. https://patchwork.ozlabs.org/project/netdev/list/?series=115399&state=* I'd like to trap all IEEE 1588 PTP Ethernet frames to CPU through etype 0x88f7. When I used current TRAP option, I found the frames were not only copied to CPU, but also forwarded to other ports. So I just made the TRAP option same with DROP option except enabling CPU_COPY_ENA in the patch. Thanks. > > > Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com> > > --- > > drivers/net/ethernet/mscc/ocelot_ace.c | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/net/ethernet/mscc/ocelot_ace.c > > b/drivers/net/ethernet/mscc/ocelot_ace.c > > index 91250f3..59ad590 100644 > > --- a/drivers/net/ethernet/mscc/ocelot_ace.c > > +++ b/drivers/net/ethernet/mscc/ocelot_ace.c > > @@ -317,9 +317,9 @@ static void is2_action_set(struct vcap_data *data, > > break; > > case OCELOT_ACL_ACTION_TRAP: > > VCAP_ACT_SET(PORT_MASK, 0x0); > > - VCAP_ACT_SET(MASK_MODE, 0x0); > > - VCAP_ACT_SET(POLICE_ENA, 0x0); > > - VCAP_ACT_SET(POLICE_IDX, 0x0); > > + VCAP_ACT_SET(MASK_MODE, 0x1); > > + VCAP_ACT_SET(POLICE_ENA, 0x1); > > + VCAP_ACT_SET(POLICE_IDX, OCELOT_POLICER_DISCARD); > This seems wrong. The policer is used to ensure that traffic are discarded, even > in the case where other users of the code has requested it to go to the CPU. > > Are you sure this is working? If it is working, then I fear we have an issue with > the DROP action which uses this to discard frames. > > > VCAP_ACT_SET(CPU_QU_NUM, 0x0); > > VCAP_ACT_SET(CPU_COPY_ENA, 0x1); > > break; > > -- > > 2.7.4 > > -- > /Allan
The 08/13/2019 02:12, Y.b. Lu wrote: > > -----Original Message----- > > From: Allan W. Nielsen <allan.nielsen@microchip.com> > > Sent: Monday, August 12, 2019 8:32 PM > > To: Y.b. Lu <yangbo.lu@nxp.com> > > Cc: netdev@vger.kernel.org; David S . Miller <davem@davemloft.net>; > > Alexandre Belloni <alexandre.belloni@bootlin.com>; Microchip Linux Driver > > Support <UNGLinuxDriver@microchip.com> > > Subject: Re: [PATCH 3/3] ocelot_ace: fix action of trap > > > > The 08/12/2019 18:48, Yangbo Lu wrote: > > > The trap action should be copying the frame to CPU and dropping it for > > > forwarding, but current setting was just copying frame to CPU. > > > > Are there any actions which do a "copy-to-cpu" and still forward the frame in > > HW? > > [Y.b. Lu] We're using Felix switch whose code hadn't been accepted by upstream. > https://patchwork.ozlabs.org/project/netdev/list/?series=115399&state=* > > I'd like to trap all IEEE 1588 PTP Ethernet frames to CPU through etype 0x88f7. > When I used current TRAP option, I found the frames were not only copied to CPU, but also forwarded to other ports. > So I just made the TRAP option same with DROP option except enabling CPU_COPY_ENA in the patch. This is still wrong to do - and it will not work for Ocelot (and I doubt it will work for your Felix target). The policer setting in the drop action ensure that the frame is dropped even if other pipe-line steps in the switch has set the copy-to-cpu flag. I think you can fix this patch my just clearing the port mask, and not set the policer. /Allan
On Tue, Aug 13, 2019 at 02:12:47AM +0000, Y.b. Lu wrote: > Hi Allan, > > > -----Original Message----- > > From: Allan W. Nielsen <allan.nielsen@microchip.com> > > Sent: Monday, August 12, 2019 8:32 PM > > To: Y.b. Lu <yangbo.lu@nxp.com> > > Cc: netdev@vger.kernel.org; David S . Miller <davem@davemloft.net>; > > Alexandre Belloni <alexandre.belloni@bootlin.com>; Microchip Linux Driver > > Support <UNGLinuxDriver@microchip.com> > > Subject: Re: [PATCH 3/3] ocelot_ace: fix action of trap > > > > The 08/12/2019 18:48, Yangbo Lu wrote: > > > The trap action should be copying the frame to CPU and dropping it for > > > forwarding, but current setting was just copying frame to CPU. > > > > Are there any actions which do a "copy-to-cpu" and still forward the frame in > > HW? > > [Y.b. Lu] We're using Felix switch whose code hadn't been accepted by upstream. > https://patchwork.ozlabs.org/project/netdev/list/?series=115399&state=* > > I'd like to trap all IEEE 1588 PTP Ethernet frames to CPU through etype 0x88f7. Is this the correct way to handle PTP for this switch? For other switches we don't need such traps. The switch itself identifies PTP frames and forwards them to the CPU so it can process them. I'm just wondering if your general approach is wrong? Andrew
Hi Allan, > -----Original Message----- > From: Allan W. Nielsen <allan.nielsen@microchip.com> > Sent: Tuesday, August 13, 2019 2:16 PM > To: Y.b. Lu <yangbo.lu@nxp.com> > Cc: netdev@vger.kernel.org; David S . Miller <davem@davemloft.net>; > Alexandre Belloni <alexandre.belloni@bootlin.com>; Microchip Linux Driver > Support <UNGLinuxDriver@microchip.com> > Subject: Re: [PATCH 3/3] ocelot_ace: fix action of trap > > The 08/13/2019 02:12, Y.b. Lu wrote: > > > -----Original Message----- > > > From: Allan W. Nielsen <allan.nielsen@microchip.com> > > > Sent: Monday, August 12, 2019 8:32 PM > > > To: Y.b. Lu <yangbo.lu@nxp.com> > > > Cc: netdev@vger.kernel.org; David S . Miller <davem@davemloft.net>; > > > Alexandre Belloni <alexandre.belloni@bootlin.com>; Microchip Linux > > > Driver Support <UNGLinuxDriver@microchip.com> > > > Subject: Re: [PATCH 3/3] ocelot_ace: fix action of trap > > > > > > The 08/12/2019 18:48, Yangbo Lu wrote: > > > > The trap action should be copying the frame to CPU and dropping it > > > > for forwarding, but current setting was just copying frame to CPU. > > > > > > Are there any actions which do a "copy-to-cpu" and still forward the > > > frame in HW? > > > > [Y.b. Lu] We're using Felix switch whose code hadn't been accepted by > upstream. > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatc > > > hwork.ozlabs.org%2Fproject%2Fnetdev%2Flist%2F%3Fseries%3D115399%26s > tat > > > e%3D*&data=02%7C01%7Cyangbo.lu%40nxp.com%7C42cd202cb17b45 > 69821708d > > > 71fb5c5de%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6370127 > 37899910 > > > 736&sdata=QnsDaWPHK9rb0XWg%2BduYEha6fuYSlv4YZdsu5f4kbfc%3D > &res > > erved=0 > > > > I'd like to trap all IEEE 1588 PTP Ethernet frames to CPU through etype > 0x88f7. > > When I used current TRAP option, I found the frames were not only copied to > CPU, but also forwarded to other ports. > > So I just made the TRAP option same with DROP option except enabling > CPU_COPY_ENA in the patch. > This is still wrong to do - and it will not work for Ocelot (and I doubt it will > work for your Felix target). > > The policer setting in the drop action ensure that the frame is dropped even if > other pipe-line steps in the switch has set the copy-to-cpu flag. > > I think you can fix this patch my just clearing the port mask, and not set the > policer. [Y.b. Lu] Sorry. I missed your previous comments on the TRAP action. With my configuration in the patch, it indeed worked. Maybe it was because "the CPU port is not touched by MASK_MODE" which I saw in RM. I will try your suggestion too. It sound more proper. Thanks. > > /Allan
Hi Andrew and Allan, > -----Original Message----- > From: Andrew Lunn <andrew@lunn.ch> > Sent: Tuesday, August 13, 2019 9:43 PM > To: Y.b. Lu <yangbo.lu@nxp.com> > Cc: Allan W. Nielsen <allan.nielsen@microchip.com>; netdev@vger.kernel.org; > David S . Miller <davem@davemloft.net>; Alexandre Belloni > <alexandre.belloni@bootlin.com>; Microchip Linux Driver Support > <UNGLinuxDriver@microchip.com> > Subject: Re: [PATCH 3/3] ocelot_ace: fix action of trap > > On Tue, Aug 13, 2019 at 02:12:47AM +0000, Y.b. Lu wrote: > > Hi Allan, > > > > > -----Original Message----- > > > From: Allan W. Nielsen <allan.nielsen@microchip.com> > > > Sent: Monday, August 12, 2019 8:32 PM > > > To: Y.b. Lu <yangbo.lu@nxp.com> > > > Cc: netdev@vger.kernel.org; David S . Miller <davem@davemloft.net>; > > > Alexandre Belloni <alexandre.belloni@bootlin.com>; Microchip Linux > > > Driver Support <UNGLinuxDriver@microchip.com> > > > Subject: Re: [PATCH 3/3] ocelot_ace: fix action of trap > > > > > > The 08/12/2019 18:48, Yangbo Lu wrote: > > > > The trap action should be copying the frame to CPU and dropping it > > > > for forwarding, but current setting was just copying frame to CPU. > > > > > > Are there any actions which do a "copy-to-cpu" and still forward the > > > frame in HW? > > > > [Y.b. Lu] We're using Felix switch whose code hadn't been accepted by > upstream. > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatc > > > hwork.ozlabs.org%2Fproject%2Fnetdev%2Flist%2F%3Fseries%3D115399%26s > tat > > > e%3D*&data=02%7C01%7Cyangbo.lu%40nxp.com%7Cfbf7f74803d040f1 > b55608d > > > 71ff41b17%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6370130 > 05597107 > > > 485&sdata=xPGDbm2XtDI0L7F5A2xLhDDtctbeqB0MFByCAlgAtJ4%3D&a > mp;reser > > ved=0 > > > > I'd like to trap all IEEE 1588 PTP Ethernet frames to CPU through etype > 0x88f7. > > Is this the correct way to handle PTP for this switch? For other switches we > don't need such traps. The switch itself identifies PTP frames and forwards > them to the CPU so it can process them. > > I'm just wondering if your general approach is wrong? [Y.b. Lu] PTP messages over Ethernet will use two multicast addresses. 01-80-C2-00-00-0E for peer delay messages. 01-1B-19-00-00-00 for other messages. But only 01-80-C2-00-00-0E could be handled by hardware filter for BPDU frames (01-80-C2-00-00-0x). For PTP messages handling, trapping them to CPU through VCAP IS2 is the suggested way by Ocelot/Felix. I have a question since you are experts. For other switches, whether they are always trapping PTP messages to CPU? Is there any common method in linux to configure switch to select trapping or just forwarding PTP messages? Like Allan's comments in new version patch. I have no idea. https://patchwork.ozlabs.org/patch/1145988/ Thanks. > > Andrew
> > -----Original Message----- > > From: Allan W. Nielsen <allan.nielsen@microchip.com> > > Sent: Tuesday, August 13, 2019 2:16 PM > > To: Y.b. Lu <yangbo.lu@nxp.com> > > Cc: netdev@vger.kernel.org; David S . Miller <davem@davemloft.net>; > > Alexandre Belloni <alexandre.belloni@bootlin.com>; Microchip Linux Driver > > Support <UNGLinuxDriver@microchip.com> > > Subject: Re: [PATCH 3/3] ocelot_ace: fix action of trap > > > > The 08/13/2019 02:12, Y.b. Lu wrote: > > > > -----Original Message----- > > > > From: Allan W. Nielsen <allan.nielsen@microchip.com> > > > > Sent: Monday, August 12, 2019 8:32 PM > > > > To: Y.b. Lu <yangbo.lu@nxp.com> > > > > Cc: netdev@vger.kernel.org; David S . Miller <davem@davemloft.net>; > > > > Alexandre Belloni <alexandre.belloni@bootlin.com>; Microchip Linux > > > > Driver Support <UNGLinuxDriver@microchip.com> > > > > Subject: Re: [PATCH 3/3] ocelot_ace: fix action of trap > > > > > > > > The 08/12/2019 18:48, Yangbo Lu wrote: > > > > > The trap action should be copying the frame to CPU and dropping it > > > > > for forwarding, but current setting was just copying frame to CPU. > > > > > > > > Are there any actions which do a "copy-to-cpu" and still forward the > > > > frame in HW? > > > > > > [Y.b. Lu] We're using Felix switch whose code hadn't been accepted by > > upstream. > > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatc > > > > > hwork.ozlabs.org%2Fproject%2Fnetdev%2Flist%2F%3Fseries%3D115399%26s > > tat > > > > > e%3D*&data=02%7C01%7Cyangbo.lu%40nxp.com%7C42cd202cb17b45 > > 69821708d > > > > > 71fb5c5de%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6370127 > > 37899910 > > > > > 736&sdata=QnsDaWPHK9rb0XWg%2BduYEha6fuYSlv4YZdsu5f4kbfc%3D > > &res > > > erved=0 > > > > > > I'd like to trap all IEEE 1588 PTP Ethernet frames to CPU through etype > > 0x88f7. > > > When I used current TRAP option, I found the frames were not only copied to > > CPU, but also forwarded to other ports. > > > So I just made the TRAP option same with DROP option except enabling > > CPU_COPY_ENA in the patch. > > This is still wrong to do - and it will not work for Ocelot (and I doubt it will > > work for your Felix target). > > > > The policer setting in the drop action ensure that the frame is dropped even if > > other pipe-line steps in the switch has set the copy-to-cpu flag. > > > > I think you can fix this patch my just clearing the port mask, and not set the > > policer. > > [Y.b. Lu] Sorry. I missed your previous comments on the TRAP action. > With my configuration in the patch, it indeed worked. Maybe it was because "the CPU port is not touched by MASK_MODE" which I saw in RM. Okay. If this is working, then you should properly test and see if the DROP action is working on your target. I do not have access to the SoC which incldues Felix, so I cannot check. > I will try your suggestion too. It sound more proper. Sounds good - thanks /Allan
Hi Y.b. and Andrew, The 08/14/2019 04:28, Y.b. Lu wrote: > > > I'd like to trap all IEEE 1588 PTP Ethernet frames to CPU through etype > > 0x88f7. > > > > Is this the correct way to handle PTP for this switch? For other switches we > > don't need such traps. The switch itself identifies PTP frames and forwards > > them to the CPU so it can process them. > > > > I'm just wondering if your general approach is wrong? > > [Y.b. Lu] PTP messages over Ethernet will use two multicast addresses. > 01-80-C2-00-00-0E for peer delay messages. Yes, and as you write, this is a BPDU which must not be forwarded (and they are not). > 01-1B-19-00-00-00 for other messages. Yes, this is a normal L2 multicast address, which by default are broadcastet. > But only 01-80-C2-00-00-0E could be handled by hardware filter for BPDU frames > (01-80-C2-00-00-0x). For PTP messages handling, trapping them to CPU through > VCAP IS2 is the suggested way by Ocelot/Felix. As I see it there are at least 3 scenarios which needs to be considered and ideally supported: 1) Operate as a PTP-unaware switch. This means that Peer-delays messages are trapped to the CPU and not handled. End-to-End PTP sessions can still run on the network as 01-1B-19-00-00-00 frames are forwarded normally. This is what we have by default today. 2) A "passive" PTP switch (in MSCC/MCHP we call this an end-to-end transparent clock) where 01-80-C2-00-00-0E frames are still trapped to the CPU (and not handled), 01-1B-19-00-00-00 frames are forwarded, but we use the TCAM to add the residence time to the correction field in Sync and Delay request messages. This is a simple mechanism which allow end-to-end PTP sessions to synchronize their time, and compensate for the variable delay caused by the switch. Compared to implement a complete boundary clock, this is much simpler, and cause a much lower work load on the CPU (even small switches may be serving many many PTP sessions). 3) Full PTP aware switch. In this mode we need all PTP frames trapped (on the ports where PTP are running) to the CPU, and we need a PTP daemon in user-space to process them in-order for things to work. I guess this is what you are trying to achieve. Eventually, I hope we can get to a point where all (and maybe more) scenarios are supported. Lets consider them case by case: 1) This is what we have today. 2) To support this, we need a SW implementation of this, and then we can add hooks to offload this in HW. We would certainly be interested in supporting this in both SW and HW. 3) It can be done via 'tc' using the trap action, but I do not know if this is the desired way of doing it. Ocelot will be using a TCAM rule to do this, which align nicely with the 'tc' approach, but other chips may be have dedicated HW for doing this. Also, in the current implementation we will be using a rule per port, and ideally we could have done it with a single rule (this is what Y.B. prepared in this patch series). I'm very much against configuring option 3 in the driver initialization, as it will prevent us from having a conforming switch if a PTP daemon is not running in user-space, and it give us very little room for supporting other ways in the future without breaking backwards compatibility. > I have a question since you are experts. I'm not really an expert on this, but I have access to some good guidance from collages knowing PTP very well :-D > For other switches, whether they are always trapping PTP messages to CPU? Good question, I could not find anything in the SW bridge forcing option 3. My understanding is that the SW bridge is implementing option 1, but I could be wrong. > Is there any common method in linux to configure switch to select trapping or > just forwarding PTP messages? You should be able to use TC for this. But due to the port vs port-mask limitation you will need to install a rule per port. I do not know if this is what others are doing, but would like to learn about that. /Allan
> [Y.b. Lu] PTP messages over Ethernet will use two multicast addresses. > 01-80-C2-00-00-0E for peer delay messages. > 01-1B-19-00-00-00 for other messages. > > But only 01-80-C2-00-00-0E could be handled by hardware filter for BPDU frames (01-80-C2-00-00-0x). > For PTP messages handling, trapping them to CPU through VCAP IS2 is the suggested way by Ocelot/Felix. > > I have a question since you are experts. There real expert is Richard Cochran <richardcochran@gmail.com>. He implemented PTP support for the mv88e6xxx devices, maintains to core code, and the linuxptp daemon. Any ptp support your post will be reviewed by him. > For other switches, whether they are always trapping PTP messages to CPU? For the mv88e6xxx family, there is a per port bit which enabled PTP. When enabled, PTP frames are recognised by the hardware and forwarded to the CPU. > Is there any common method in linux to configure switch to select > trapping or just forwarding PTP messages? The best answer to that is to look at other switch driver which implement ptp. The ptp core expects a ptp_clock_info structure, and one of its members is 'enable'. Andrew
On Wed, Aug 14, 2019 at 10:57:12AM +0200, Allan W. Nielsen wrote: > Hi Y.b. and Andrew, > > The 08/14/2019 04:28, Y.b. Lu wrote: > > > > I'd like to trap all IEEE 1588 PTP Ethernet frames to CPU through etype > > > 0x88f7. > > > > > > Is this the correct way to handle PTP for this switch? For other switches we > > > don't need such traps. The switch itself identifies PTP frames and forwards > > > them to the CPU so it can process them. > > > > > > I'm just wondering if your general approach is wrong? > > > > [Y.b. Lu] PTP messages over Ethernet will use two multicast addresses. > > 01-80-C2-00-00-0E for peer delay messages. > Yes, and as you write, this is a BPDU which must not be forwarded (and they are > not). > > > 01-1B-19-00-00-00 for other messages. > Yes, this is a normal L2 multicast address, which by default are broadcastet. > > > But only 01-80-C2-00-00-0E could be handled by hardware filter for BPDU frames > > (01-80-C2-00-00-0x). For PTP messages handling, trapping them to CPU through > > VCAP IS2 is the suggested way by Ocelot/Felix. Hi Allan The typical userspace for this is linuxptp. It implements Boundary Clock (BC), Ordinary Clock (OC) and Transparent Clock (TC). On switches, it works great for L2 PTP. But it has architectural issues for L3 PTP when used with a bridge. I've no idea if Richard is fixing this. > 3) It can be done via 'tc' using the trap action, but I do not know if this is > the desired way of doing it. No, it is not. It could be the way you the implement ptp_clock_info.enable() does the same as what TC could do, but TC itself is not used, it should all be internal to the driver. And you might also want to consider hiding such rules from TC, otherwise the user might remove them and things break. Andrew
Hi Andrew and Allan, I add Richard in email for help and some suggestions. And please see my comments inline. Thanks. Hi Richard, We are discussing problem of PTP message trapping to CPU on switch. Hope for your suggestions since you are the expert. Here are the two versions patch-set. V2: https://patchwork.ozlabs.org/project/netdev/list/?series=124713&state=* V1: https://patchwork.ozlabs.org/project/netdev/list/?series=124563&state=* Thanks in advance :) > -----Original Message----- > From: Andrew Lunn <andrew@lunn.ch> > Sent: Wednesday, August 14, 2019 9:52 PM > To: Allan W. Nielsen <allan.nielsen@microchip.com> > Cc: Y.b. Lu <yangbo.lu@nxp.com>; netdev@vger.kernel.org; David S . Miller > <davem@davemloft.net>; Alexandre Belloni <alexandre.belloni@bootlin.com>; > Microchip Linux Driver Support <UNGLinuxDriver@microchip.com> > Subject: Re: [PATCH 3/3] ocelot_ace: fix action of trap > > On Wed, Aug 14, 2019 at 10:57:12AM +0200, Allan W. Nielsen wrote: > > Hi Y.b. and Andrew, > > > > The 08/14/2019 04:28, Y.b. Lu wrote: > > > > > I'd like to trap all IEEE 1588 PTP Ethernet frames to CPU > > > > > through etype > > > > 0x88f7. > > > > > > > > Is this the correct way to handle PTP for this switch? For other > > > > switches we don't need such traps. The switch itself identifies > > > > PTP frames and forwards them to the CPU so it can process them. > > > > > > > > I'm just wondering if your general approach is wrong? > > > > > > [Y.b. Lu] PTP messages over Ethernet will use two multicast addresses. > > > 01-80-C2-00-00-0E for peer delay messages. > > Yes, and as you write, this is a BPDU which must not be forwarded (and > > they are not). > > > > > 01-1B-19-00-00-00 for other messages. > > Yes, this is a normal L2 multicast address, which by default are broadcastet. > > > > > But only 01-80-C2-00-00-0E could be handled by hardware filter for > > > BPDU frames (01-80-C2-00-00-0x). For PTP messages handling, > > > trapping them to CPU through VCAP IS2 is the suggested way by > Ocelot/Felix. > > Hi Allan > > The typical userspace for this is linuxptp. It implements Boundary Clock (BC), > Ordinary Clock (OC) and Transparent Clock (TC). On switches, it works great for > L2 PTP. But it has architectural issues for L3 PTP when used with a bridge. I've > no idea if Richard is fixing this. [Y.b. Lu] Right. For the 3 scenarios Allan listed, actually I think usually the first and the second wouldn't be used if user needs 1588 synchronization. #1 scenario, since it's PTP unaware, asymmetric delay will be introduced and it couldn't ensure sync performance. #2 scenario, this has too much limitation. The switch hardware could only be configured as two-step end-to-end transparent clock in hardware. For other clock types, it requires CPU handling and ptp software. In addition, ptp software takes very few resources. So the desired scenario for 1588 synchronization should be #3 scenario. > > > 3) It can be done via 'tc' using the trap action, but I do not know if this is > > the desired way of doing it. > > No, it is not. It could be the way you the implement > ptp_clock_info.enable() does the same as what TC could do, but TC itself is not > used, it should all be internal to the driver. And you might also want to > consider hiding such rules from TC, otherwise the user might remove them and > things break. [Y.b. Lu] ptp_clock_info.enable() ? As I understand, PTP clock driver is only for ptp clock operations, not for networking. I would have intended to send PTP trapping rule patch for discussion after Felix driver is ready on upstream. Let me just send TRAP option fix-up patch in next version. Will rework and send PTP trapping rule patch once Felix driver is accepted by upstream. But I'd like to gather suggestions on that. Thanks a lot. > > Andrew
diff --git a/drivers/net/ethernet/mscc/ocelot_ace.c b/drivers/net/ethernet/mscc/ocelot_ace.c index 91250f3..59ad590 100644 --- a/drivers/net/ethernet/mscc/ocelot_ace.c +++ b/drivers/net/ethernet/mscc/ocelot_ace.c @@ -317,9 +317,9 @@ static void is2_action_set(struct vcap_data *data, break; case OCELOT_ACL_ACTION_TRAP: VCAP_ACT_SET(PORT_MASK, 0x0); - VCAP_ACT_SET(MASK_MODE, 0x0); - VCAP_ACT_SET(POLICE_ENA, 0x0); - VCAP_ACT_SET(POLICE_IDX, 0x0); + VCAP_ACT_SET(MASK_MODE, 0x1); + VCAP_ACT_SET(POLICE_ENA, 0x1); + VCAP_ACT_SET(POLICE_IDX, OCELOT_POLICER_DISCARD); VCAP_ACT_SET(CPU_QU_NUM, 0x0); VCAP_ACT_SET(CPU_COPY_ENA, 0x1); break;
The trap action should be copying the frame to CPU and dropping it for forwarding, but current setting was just copying frame to CPU. Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com> --- drivers/net/ethernet/mscc/ocelot_ace.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)