| Message ID | 20190813025214.18601-5-yangbo.lu@nxp.com |
|---|---|
| State | Changes Requested |
| Delegated to: | David Miller |
| Headers | show |
| Series | ocelot: support PTP Ethernet frames trapping | expand |
The 08/13/2019 10:52, Yangbo Lu wrote: > All the PTP messages over Ethernet have etype 0x88f7 on them. > Use etype as the key to trap PTP messages. > > Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com> > --- > Changes for v2: > - Added this patch. > --- > drivers/net/ethernet/mscc/ocelot.c | 28 ++++++++++++++++++++++++++++ > 1 file changed, 28 insertions(+) > > diff --git a/drivers/net/ethernet/mscc/ocelot.c b/drivers/net/ethernet/mscc/ocelot.c > index 6932e61..40f4e0d 100644 > --- a/drivers/net/ethernet/mscc/ocelot.c > +++ b/drivers/net/ethernet/mscc/ocelot.c > @@ -1681,6 +1681,33 @@ int ocelot_probe_port(struct ocelot *ocelot, u8 port, > } > EXPORT_SYMBOL(ocelot_probe_port); > > +static int ocelot_ace_add_ptp_rule(struct ocelot *ocelot) > +{ > + struct ocelot_ace_rule *rule; > + > + rule = kzalloc(sizeof(*rule), GFP_KERNEL); > + if (!rule) > + return -ENOMEM; > + > + /* Entry for PTP over Ethernet (etype 0x88f7) > + * Action: trap to CPU port > + */ > + rule->ocelot = ocelot; > + rule->prio = 1; > + rule->type = OCELOT_ACE_TYPE_ETYPE; > + /* Available on all ingress port except CPU port */ > + rule->ingress_port = ~BIT(ocelot->num_phys_ports); > + rule->dmac_mc = OCELOT_VCAP_BIT_1; > + rule->frame.etype.etype.value[0] = 0x88; > + rule->frame.etype.etype.value[1] = 0xf7; > + rule->frame.etype.etype.mask[0] = 0xff; > + rule->frame.etype.etype.mask[1] = 0xff; > + rule->action = OCELOT_ACL_ACTION_TRAP; > + > + ocelot_ace_rule_offload_add(rule); > + return 0; > +} > + > int ocelot_init(struct ocelot *ocelot) > { > u32 port; > @@ -1708,6 +1735,7 @@ int ocelot_init(struct ocelot *ocelot) > ocelot_mact_init(ocelot); > ocelot_vlan_init(ocelot); > ocelot_ace_init(ocelot); > + ocelot_ace_add_ptp_rule(ocelot); > > for (port = 0; port < ocelot->num_phys_ports; port++) { > /* Clear all counters (5 groups) */ This seems really wrong to me, and much too hard-coded... What if I want to forward the PTP frames to be forwarded like a normal non-aware PTP switch? What if do not want this on all ports? If you do not have an application behind this implementing a boundary or transparent clock, then you are breaking PTP on the network. /Allan
Hi Allan, > -----Original Message----- > From: Allan W . Nielsen <allan.nielsen@microchip.com> > Sent: Tuesday, August 13, 2019 2:25 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: [v2, 4/4] ocelot: add VCAP IS2 rule to trap PTP Ethernet frames > > The 08/13/2019 10:52, Yangbo Lu wrote: > > All the PTP messages over Ethernet have etype 0x88f7 on them. > > Use etype as the key to trap PTP messages. > > > > Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com> > > --- > > Changes for v2: > > - Added this patch. > > --- > > drivers/net/ethernet/mscc/ocelot.c | 28 ++++++++++++++++++++++++++++ > > 1 file changed, 28 insertions(+) > > > > diff --git a/drivers/net/ethernet/mscc/ocelot.c > > b/drivers/net/ethernet/mscc/ocelot.c > > index 6932e61..40f4e0d 100644 > > --- a/drivers/net/ethernet/mscc/ocelot.c > > +++ b/drivers/net/ethernet/mscc/ocelot.c > > @@ -1681,6 +1681,33 @@ int ocelot_probe_port(struct ocelot *ocelot, u8 > > port, } EXPORT_SYMBOL(ocelot_probe_port); > > > > +static int ocelot_ace_add_ptp_rule(struct ocelot *ocelot) { > > + struct ocelot_ace_rule *rule; > > + > > + rule = kzalloc(sizeof(*rule), GFP_KERNEL); > > + if (!rule) > > + return -ENOMEM; > > + > > + /* Entry for PTP over Ethernet (etype 0x88f7) > > + * Action: trap to CPU port > > + */ > > + rule->ocelot = ocelot; > > + rule->prio = 1; > > + rule->type = OCELOT_ACE_TYPE_ETYPE; > > + /* Available on all ingress port except CPU port */ > > + rule->ingress_port = ~BIT(ocelot->num_phys_ports); > > + rule->dmac_mc = OCELOT_VCAP_BIT_1; > > + rule->frame.etype.etype.value[0] = 0x88; > > + rule->frame.etype.etype.value[1] = 0xf7; > > + rule->frame.etype.etype.mask[0] = 0xff; > > + rule->frame.etype.etype.mask[1] = 0xff; > > + rule->action = OCELOT_ACL_ACTION_TRAP; > > + > > + ocelot_ace_rule_offload_add(rule); > > + return 0; > > +} > > + > > int ocelot_init(struct ocelot *ocelot) { > > u32 port; > > @@ -1708,6 +1735,7 @@ int ocelot_init(struct ocelot *ocelot) > > ocelot_mact_init(ocelot); > > ocelot_vlan_init(ocelot); > > ocelot_ace_init(ocelot); > > + ocelot_ace_add_ptp_rule(ocelot); > > > > for (port = 0; port < ocelot->num_phys_ports; port++) { > > /* Clear all counters (5 groups) */ > This seems really wrong to me, and much too hard-coded... > > What if I want to forward the PTP frames to be forwarded like a normal > non-aware PTP switch? [Y.b. Lu] As Andrew said, other switches could identify PTP messages and forward to CPU for processing. https://patchwork.ozlabs.org/patch/1145627/ I'm also wondering whether there is common method in linux to address your questions. If no, I think trapping all PTP messages on all ports to CPU could be used for now. If users require PTP synchronization, they actually don’t want a non-aware PTP switch. I once see other ocelot code configure ptp trap rules in ioctl timestamping setting. But I don’t think it's proper either. Enable timestamping doesn’t mean we want to trap PTP messages. > > What if do not want this on all ports? [Y.b. Lu] Actually I don’t think there should be difference of handling PTP messages on each port. You don’t need to run PTP protocol application on the specific port if you don’t want. > > If you do not have an application behind this implementing a boundary or > transparent clock, then you are breaking PTP on the network. [Y.b. Lu] You're right. But actually for PTP network, all PTP devices should run PTP protocol on it. Of course, it's better to have a way to configure it as non-aware PTP switch. > > /Allan
The 08/14/2019 04:56, Y.b. Lu wrote: > > -----Original Message----- > > From: Allan W . Nielsen <allan.nielsen@microchip.com> > > Sent: Tuesday, August 13, 2019 2:25 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: [v2, 4/4] ocelot: add VCAP IS2 rule to trap PTP Ethernet frames > > > > The 08/13/2019 10:52, Yangbo Lu wrote: > > > All the PTP messages over Ethernet have etype 0x88f7 on them. > > > Use etype as the key to trap PTP messages. > > > > > > Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com> > > > --- > > > Changes for v2: > > > - Added this patch. > > > --- > > > drivers/net/ethernet/mscc/ocelot.c | 28 ++++++++++++++++++++++++++++ > > > 1 file changed, 28 insertions(+) > > > > > > diff --git a/drivers/net/ethernet/mscc/ocelot.c > > > b/drivers/net/ethernet/mscc/ocelot.c > > > index 6932e61..40f4e0d 100644 > > > --- a/drivers/net/ethernet/mscc/ocelot.c > > > +++ b/drivers/net/ethernet/mscc/ocelot.c > > > @@ -1681,6 +1681,33 @@ int ocelot_probe_port(struct ocelot *ocelot, u8 > > > port, } EXPORT_SYMBOL(ocelot_probe_port); > > > > > > +static int ocelot_ace_add_ptp_rule(struct ocelot *ocelot) { > > > + struct ocelot_ace_rule *rule; > > > + > > > + rule = kzalloc(sizeof(*rule), GFP_KERNEL); > > > + if (!rule) > > > + return -ENOMEM; > > > + > > > + /* Entry for PTP over Ethernet (etype 0x88f7) > > > + * Action: trap to CPU port > > > + */ > > > + rule->ocelot = ocelot; > > > + rule->prio = 1; > > > + rule->type = OCELOT_ACE_TYPE_ETYPE; > > > + /* Available on all ingress port except CPU port */ > > > + rule->ingress_port = ~BIT(ocelot->num_phys_ports); > > > + rule->dmac_mc = OCELOT_VCAP_BIT_1; > > > + rule->frame.etype.etype.value[0] = 0x88; > > > + rule->frame.etype.etype.value[1] = 0xf7; > > > + rule->frame.etype.etype.mask[0] = 0xff; > > > + rule->frame.etype.etype.mask[1] = 0xff; > > > + rule->action = OCELOT_ACL_ACTION_TRAP; > > > + > > > + ocelot_ace_rule_offload_add(rule); > > > + return 0; > > > +} > > > + > > > int ocelot_init(struct ocelot *ocelot) { > > > u32 port; > > > @@ -1708,6 +1735,7 @@ int ocelot_init(struct ocelot *ocelot) > > > ocelot_mact_init(ocelot); > > > ocelot_vlan_init(ocelot); > > > ocelot_ace_init(ocelot); > > > + ocelot_ace_add_ptp_rule(ocelot); > > > > > > for (port = 0; port < ocelot->num_phys_ports; port++) { > > > /* Clear all counters (5 groups) */ > > This seems really wrong to me, and much too hard-coded... > > > > What if I want to forward the PTP frames to be forwarded like a normal > > non-aware PTP switch? > > [Y.b. Lu] As Andrew said, other switches could identify PTP messages and forward to CPU for processing. > https://patchwork.ozlabs.org/patch/1145627/ Yes, it would be good to see some exampels to understand this better. > I'm also wondering whether there is common method in linux to address your questions. Me too. > If no, I think trapping all PTP messages on all ports to CPU could be used for now. > If users require PTP synchronization, they actually don’t want a non-aware PTP switch. Can we continue this discussion in the other thread where I listed the 3 scenarios? > I once see other ocelot code configure ptp trap rules in ioctl timestamping > setting. But I don’t think it's proper either. Enable timestamping doesn’t > mean we want to trap PTP messages. Where did you see this? The effort in [1] is just about the time-stamping and does not really consider the bridge part of it, and it should not be installing any TCAM rules (I believe it did in earlier versions, but this has been changed). [1] https://patchwork.ozlabs.org/patch/1145777/ > > What if do not want this on all ports? > [Y.b. Lu] Actually I don’t think there should be difference of handling PTP messages on each port. > You don’t need to run PTP protocol application on the specific port if you don’t want. What if you want some vlans or some ports to be PTP unaware, and other to be PTP aware. > > If you do not have an application behind this implementing a boundary or > > transparent clock, then you are breaking PTP on the network. > [Y.b. Lu] You're right. But actually for PTP network, all PTP devices should run PTP protocol on it. > Of course, it's better to have a way to configure it as non-aware PTP switch. I think we agree. In my point of view, it is the PTP daemon who should configure frames to be trapped. Then the switch will be PTP unaware until the PTP daemon starts up and is ready to make it aware. If we put it in the init function, then it will be of PTP broken until the PTP daemon starts. /Allan
Hi Allan, > -----Original Message----- > From: Allan W . Nielsen <allan.nielsen@microchip.com> > Sent: Wednesday, August 14, 2019 5:17 PM > To: Y.b. Lu <yangbo.lu@nxp.com> > Cc: Andrew Lunn <andrew@lunn.ch>; 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: [v2, 4/4] ocelot: add VCAP IS2 rule to trap PTP Ethernet frames > > The 08/14/2019 04:56, Y.b. Lu wrote: > > > -----Original Message----- > > > From: Allan W . Nielsen <allan.nielsen@microchip.com> > > > Sent: Tuesday, August 13, 2019 2:25 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: [v2, 4/4] ocelot: add VCAP IS2 rule to trap PTP > > > Ethernet frames > > > > > > The 08/13/2019 10:52, Yangbo Lu wrote: > > > > All the PTP messages over Ethernet have etype 0x88f7 on them. > > > > Use etype as the key to trap PTP messages. > > > > > > > > Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com> [...] > Can we continue this discussion in the other thread where I listed the 3 > scenarios? [Y.b. Lu] Sure. Let's discuss in that thread. https://patchwork.ozlabs.org/patch/1145627/ [...] > > > What if do not want this on all ports? > > [Y.b. Lu] Actually I don’t think there should be difference of handling PTP > messages on each port. > > You don’t need to run PTP protocol application on the specific port if you > don’t want. > What if you want some vlans or some ports to be PTP unaware, and other to > be PTP aware. [Y.b. Lu] Actually I couldn’t find reasons why make some ports PTP unaware, if there is software stack for PTP aware... > > > > If you do not have an application behind this implementing a > > > boundary or transparent clock, then you are breaking PTP on the network. > > [Y.b. Lu] You're right. But actually for PTP network, all PTP devices should run > PTP protocol on it. > > Of course, it's better to have a way to configure it as non-aware PTP switch. > I think we agree. > > In my point of view, it is the PTP daemon who should configure frames to be > trapped. Then the switch will be PTP unaware until the PTP daemon starts up > and is ready to make it aware. > > If we put it in the init function, then it will be of PTP broken until the PTP > daemon starts. > > /Allan
> [Y.b. Lu] Actually I couldn’t find reasons why make some ports PTP > unaware, if there is software stack for PTP aware... Maybe because i have not yet done apt-get install linuxptp $EDITOR /etc/defaults/ptp4l systemctl restart ptp4l Just because it exists does not mean it is installed. Andrew
diff --git a/drivers/net/ethernet/mscc/ocelot.c b/drivers/net/ethernet/mscc/ocelot.c index 6932e61..40f4e0d 100644 --- a/drivers/net/ethernet/mscc/ocelot.c +++ b/drivers/net/ethernet/mscc/ocelot.c @@ -1681,6 +1681,33 @@ int ocelot_probe_port(struct ocelot *ocelot, u8 port, } EXPORT_SYMBOL(ocelot_probe_port); +static int ocelot_ace_add_ptp_rule(struct ocelot *ocelot) +{ + struct ocelot_ace_rule *rule; + + rule = kzalloc(sizeof(*rule), GFP_KERNEL); + if (!rule) + return -ENOMEM; + + /* Entry for PTP over Ethernet (etype 0x88f7) + * Action: trap to CPU port + */ + rule->ocelot = ocelot; + rule->prio = 1; + rule->type = OCELOT_ACE_TYPE_ETYPE; + /* Available on all ingress port except CPU port */ + rule->ingress_port = ~BIT(ocelot->num_phys_ports); + rule->dmac_mc = OCELOT_VCAP_BIT_1; + rule->frame.etype.etype.value[0] = 0x88; + rule->frame.etype.etype.value[1] = 0xf7; + rule->frame.etype.etype.mask[0] = 0xff; + rule->frame.etype.etype.mask[1] = 0xff; + rule->action = OCELOT_ACL_ACTION_TRAP; + + ocelot_ace_rule_offload_add(rule); + return 0; +} + int ocelot_init(struct ocelot *ocelot) { u32 port; @@ -1708,6 +1735,7 @@ int ocelot_init(struct ocelot *ocelot) ocelot_mact_init(ocelot); ocelot_vlan_init(ocelot); ocelot_ace_init(ocelot); + ocelot_ace_add_ptp_rule(ocelot); for (port = 0; port < ocelot->num_phys_ports; port++) { /* Clear all counters (5 groups) */
All the PTP messages over Ethernet have etype 0x88f7 on them. Use etype as the key to trap PTP messages. Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com> --- Changes for v2: - Added this patch. --- drivers/net/ethernet/mscc/ocelot.c | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+)