Re: [PATCH net-next 0/5] PTP support for the SJA1105 DSA driver
On Sat, Jun 01, 2019 at 03:06:59PM +0300, Vladimir Oltean wrote: > PTP frames will reconstruct the full timestamp without waiting for any > meta (they are the meta), while other MAC-trapped frames (STP etc) > will just carry a meaningless skb->cb when passed up the stack. > In retrospect, it would have been amazing if the switch gave me the > meta frames *before* the actual link-local frames that needed the > timestamp. I didn't follow everything you wrote, but it sounds like you are on the right track! Thanks, Richard
Re: [PATCH net-next 0/5] PTP support for the SJA1105 DSA driver
On Sat, Jun 01, 2019 at 01:31:34PM +0300, Vladimir Oltean wrote:
> If I dress the meta frame into a PTP frame (btw is there any
> preferable event message for this purpose?)
I would just make a L2 PTP event message from a specific source
address, just like the phyter does.
Use Ethertype ETH_P_1588 (0x88f7), and make sure the "general" bit
(0x8) of the messageType field (the first payload byte, at offset 14)
is clear.
dp83640.c uses a magic source address to identify a time stamping
status frame:
static u8 status_frame_src[6] = { 0x08, 0x00, 0x17, 0x0B, 0x6B, 0x0F };
HTH,
Richard
Re: [PATCH net-next 0/5] PTP support for the SJA1105 DSA driver
On Sat, 1 Jun 2019 at 13:31, Vladimir Oltean wrote: > > On Sat, 1 Jun 2019 at 08:07, Richard Cochran wrote: > > > > On Fri, May 31, 2019 at 09:12:03PM +0300, Vladimir Oltean wrote: > > > It won't work unless I make changes to dsa_switch_rcv. > > > > Or to the tagging code. > > > > > Right now taggers can only return a pointer to the skb, or NULL, case > > > in which DSA will free it. > > > > The tagger can re-write the skb. Why not reform it into a PTP frame? > > This clever trick is what the phyter does in hardware. See dp83640.c. > > > > I think you're missing the point here. > If I dress the meta frame into a PTP frame (btw is there any > preferable event message for this purpose?) then sure, I'll make > dsa_skb_defer_rx_timestamp call my .port_rxtstamp and I can e.g. move > my state machine there. > The problem is that in the current DSA structure, I'll still have less > timestampable frames waiting for a meta frame than meta frames > themselves. This is because not all frames that the switch takes an RX > timestamp for will make it to my .port_rxtstamp (in fact that is what > my .can_timestamp patch changes). I can put the timestampable frame in > a 1-entry wait queue which I'll deplete upon arrival of the first meta > frame, but when I get meta frames and the wait queue is empty, it can > mean multiple things: either DSA didn't care about this timestamp > (ok), or the timestampable frame got reordered or dropped by the MAC, > or what have you (not ok). So I can't exclude the possibility that the > meta frame was holding a relevant timestamp. > Sure, I can dress the meta frame into whatever the previous > MAC-trapped frame was (PTP or not) and then I'll have .port_rxtstamp > function see a 1-to-1 correspondence with meta frames in case > everything works fine. But then I'll have non-PTP meta frames leaking > up the stack... > Actually maybe this is exactly what you meant and I didn't think it through. If RX timestamping is enabled, then I can just copy all MAC-trapped frames to a private skb in a per-driver data structure, and have DSA drop them. Then when their meta frame arrives, I can just morph them into what the previous frame was, just that now I'm also holding the partial timestamp in skb->cb. PTP frames will reconstruct the full timestamp without waiting for any meta (they are the meta), while other MAC-trapped frames (STP etc) will just carry a meaningless skb->cb when passed up the stack. In retrospect, it would have been amazing if the switch gave me the meta frames *before* the actual link-local frames that needed the timestamp. Thanks! -Vladimir > Regards, > -Vladimir > > > Thanks, > > Richard
Re: [PATCH net-next 0/5] PTP support for the SJA1105 DSA driver
On Sat, 1 Jun 2019 at 08:07, Richard Cochran wrote: > > On Fri, May 31, 2019 at 09:12:03PM +0300, Vladimir Oltean wrote: > > It won't work unless I make changes to dsa_switch_rcv. > > Or to the tagging code. > > > Right now taggers can only return a pointer to the skb, or NULL, case > > in which DSA will free it. > > The tagger can re-write the skb. Why not reform it into a PTP frame? > This clever trick is what the phyter does in hardware. See dp83640.c. > I think you're missing the point here. If I dress the meta frame into a PTP frame (btw is there any preferable event message for this purpose?) then sure, I'll make dsa_skb_defer_rx_timestamp call my .port_rxtstamp and I can e.g. move my state machine there. The problem is that in the current DSA structure, I'll still have less timestampable frames waiting for a meta frame than meta frames themselves. This is because not all frames that the switch takes an RX timestamp for will make it to my .port_rxtstamp (in fact that is what my .can_timestamp patch changes). I can put the timestampable frame in a 1-entry wait queue which I'll deplete upon arrival of the first meta frame, but when I get meta frames and the wait queue is empty, it can mean multiple things: either DSA didn't care about this timestamp (ok), or the timestampable frame got reordered or dropped by the MAC, or what have you (not ok). So I can't exclude the possibility that the meta frame was holding a relevant timestamp. Sure, I can dress the meta frame into whatever the previous MAC-trapped frame was (PTP or not) and then I'll have .port_rxtstamp function see a 1-to-1 correspondence with meta frames in case everything works fine. But then I'll have non-PTP meta frames leaking up the stack... Regards, -Vladimir > Thanks, > Richard
Re: [PATCH net-next 0/5] PTP support for the SJA1105 DSA driver
On Fri, May 31, 2019 at 09:12:03PM +0300, Vladimir Oltean wrote: > It won't work unless I make changes to dsa_switch_rcv. Or to the tagging code. > Right now taggers can only return a pointer to the skb, or NULL, case > in which DSA will free it. The tagger can re-write the skb. Why not reform it into a PTP frame? This clever trick is what the phyter does in hardware. See dp83640.c. Thanks, Richard
Re: [PATCH net-next 0/5] PTP support for the SJA1105 DSA driver
On Fri, May 31, 2019 at 07:16:17PM +0300, Vladimir Oltean wrote: > But now comes the question on what to do on error cases - the meta > frame didn't arrive. Should I just drop the skb waiting for it? Yes, that is what other drivers do. > Right now I "goto rcv_anyway" - which linuxptp doesn't like btw. The application sees the missing time stamp and prints a warning message. This is IHMO the right thing to do, so that the user is made aware of the degradation of the synchronization. Thanks, Richard
Re: [PATCH net-next 0/5] PTP support for the SJA1105 DSA driver
On Fri, 31 May 2019 at 19:16, Vladimir Oltean wrote: > > On Fri, 31 May 2019 at 19:09, Richard Cochran > wrote: > > > > On Fri, May 31, 2019 at 06:23:34PM +0300, Vladimir Oltean wrote: > > > You mean to queue it and subvert DSA's own RX timestamping callback? > > > > No, use the callback. > > > > > Why would I do that? Just so as not to introduce my .can_timestamp > > > callback? > > > > Right, the .can_timestamp is unneeded, AFAICT. > > > > > > Now I'm starting to understand your series. I think it can be done in > > > > simpler way... > > > > > > > > sja1105_rcv_meta_state_machine - can and should be at the driver level > > > > and not at the port level. > > > > > > > > > > Can: yes. Should: why? > > > > To keep it simple and robust. > > > > > One important aspect makes this need be a little bit more complicated: > > > reconstructing these RX timestamps. > > > You see, there is a mutex on the SPI bus, so in practice I do need the > > > sja1105_port_rxtstamp_work for exactly this purpose - to read the > > > timestamping clock over SPI. > > > > Sure. But you schedule the work after a META frame. And no busy > > waiting is needed. > > Ok, I suppose this could work. > But now comes the question on what to do on error cases - the meta > frame didn't arrive. Should I just drop the skb waiting for it? Right > now I "goto rcv_anyway" - which linuxptp doesn't like btw. > Actually I've been there before, just forgot it. It won't work unless I make changes to dsa_switch_rcv. Right now taggers can only return a pointer to the skb, or NULL, case in which DSA will free it. I'd need a mechanism to signal DSA that I'm holding up the skb for a little bit more time, then re-engage the dsa_switch_rcv path once the meta frame arrived. > > > > Thanks, > > Richard
Re: [PATCH net-next 0/5] PTP support for the SJA1105 DSA driver
On Fri, 31 May 2019 at 19:09, Richard Cochran wrote: > > On Fri, May 31, 2019 at 06:23:34PM +0300, Vladimir Oltean wrote: > > You mean to queue it and subvert DSA's own RX timestamping callback? > > No, use the callback. > > > Why would I do that? Just so as not to introduce my .can_timestamp > > callback? > > Right, the .can_timestamp is unneeded, AFAICT. > > > > Now I'm starting to understand your series. I think it can be done in > > > simpler way... > > > > > > sja1105_rcv_meta_state_machine - can and should be at the driver level > > > and not at the port level. > > > > > > > Can: yes. Should: why? > > To keep it simple and robust. > > > One important aspect makes this need be a little bit more complicated: > > reconstructing these RX timestamps. > > You see, there is a mutex on the SPI bus, so in practice I do need the > > sja1105_port_rxtstamp_work for exactly this purpose - to read the > > timestamping clock over SPI. > > Sure. But you schedule the work after a META frame. And no busy > waiting is needed. Ok, I suppose this could work. But now comes the question on what to do on error cases - the meta frame didn't arrive. Should I just drop the skb waiting for it? Right now I "goto rcv_anyway" - which linuxptp doesn't like btw. > > Thanks, > Richard
Re: [PATCH net-next 0/5] PTP support for the SJA1105 DSA driver
On Fri, May 31, 2019 at 06:23:34PM +0300, Vladimir Oltean wrote: > You mean to queue it and subvert DSA's own RX timestamping callback? No, use the callback. > Why would I do that? Just so as not to introduce my .can_timestamp > callback? Right, the .can_timestamp is unneeded, AFAICT. > > Now I'm starting to understand your series. I think it can be done in > > simpler way... > > > > sja1105_rcv_meta_state_machine - can and should be at the driver level > > and not at the port level. > > > > Can: yes. Should: why? To keep it simple and robust. > One important aspect makes this need be a little bit more complicated: > reconstructing these RX timestamps. > You see, there is a mutex on the SPI bus, so in practice I do need the > sja1105_port_rxtstamp_work for exactly this purpose - to read the > timestamping clock over SPI. Sure. But you schedule the work after a META frame. And no busy waiting is needed. Thanks, Richard
Re: [PATCH net-next 0/5] PTP support for the SJA1105 DSA driver
On Fri, 31 May 2019 at 18:11, Richard Cochran wrote: > > On Fri, May 31, 2019 at 05:27:15PM +0300, Vladimir Oltean wrote: > > On Fri, 31 May 2019 at 17:08, Richard Cochran > > wrote: > > > This can be done simply using a data structure in the driver with an > > > appropriate locking mechanism. Then you don't have to worry which > > > core the driver code runs on. > > > > > > > Actually you do. DSA is special because it is not the first net device > > in the RX path that processes the frames. Something needs to be done > > on the master port. > > Before you said, > > the switch in its great wisdom mangles bytes 01-1B-19-xx-xx-00 > of the DMAC to place the switch id and source port there (a > rudimentary tagging mechanism). > > So why not simply save each frame in a per-switch/port data structure? > You mean to queue it and subvert DSA's own RX timestamping callback? Why would I do that? Just so as not to introduce my .can_timestamp callback? > Now I'm starting to understand your series. I think it can be done in > simpler way... > > sja1105_rcv_meta_state_machine - can and should be at the driver level > and not at the port level. > Can: yes. Should: why? > sja1105_port_rxtstamp_work - isn't needed at all. > > How about this? > > 1. When the driver receives a deferred PTP frame, save it into a >per-switch,port slot at the driver (not port) level. > > 2. When the driver receives a META frame, match it to the >per-switch,port slot. If there is a PTP frame in that slot, then >deliver it with the time stamp from the META frame. > One important aspect makes this need be a little bit more complicated: reconstructing these RX timestamps. You see, there is a mutex on the SPI bus, so in practice I do need the sja1105_port_rxtstamp_work for exactly this purpose - to read the timestamping clock over SPI. > Thanks, > Richard
Re: [PATCH net-next 0/5] PTP support for the SJA1105 DSA driver
On Fri, May 31, 2019 at 08:11:51AM -0700, Richard Cochran wrote: > > 1. When the driver receives a deferred PTP frame, save it into a >per-switch,port slot at the driver (not port) level. > > 2. When the driver receives a META frame, match it to the >per-switch,port slot. If there is a PTP frame in that slot, then >deliver it with the time stamp from the META frame. Actually, since the switch guarantees strict ordering, you don't need multiple slots. You only need to save one Rx'd PTP frame with its switch-id and port-id. Thanks, Richard
Re: [PATCH net-next 0/5] PTP support for the SJA1105 DSA driver
On Fri, May 31, 2019 at 05:27:15PM +0300, Vladimir Oltean wrote: > On Fri, 31 May 2019 at 17:08, Richard Cochran > wrote: > > This can be done simply using a data structure in the driver with an > > appropriate locking mechanism. Then you don't have to worry which > > core the driver code runs on. > > > > Actually you do. DSA is special because it is not the first net device > in the RX path that processes the frames. Something needs to be done > on the master port. Before you said, the switch in its great wisdom mangles bytes 01-1B-19-xx-xx-00 of the DMAC to place the switch id and source port there (a rudimentary tagging mechanism). So why not simply save each frame in a per-switch/port data structure? Now I'm starting to understand your series. I think it can be done in simpler way... sja1105_rcv_meta_state_machine - can and should be at the driver level and not at the port level. sja1105_port_rxtstamp_work - isn't needed at all. How about this? 1. When the driver receives a deferred PTP frame, save it into a per-switch,port slot at the driver (not port) level. 2. When the driver receives a META frame, match it to the per-switch,port slot. If there is a PTP frame in that slot, then deliver it with the time stamp from the META frame. Thanks, Richard
Re: [PATCH net-next 0/5] PTP support for the SJA1105 DSA driver
On Fri, 31 May 2019 at 17:08, Richard Cochran wrote: > > On Fri, May 31, 2019 at 04:23:24PM +0300, Vladimir Oltean wrote: > > The switch has internal logic to not send any other frame to the CPU > > between a link-local and a meta frame. > > So this is guarantied by the switch? What happens when multiple PTP > frames arrive at the same time on different ports? Does the switch > buffer them and ensure strict ordering at the CPU port? > > In any case, the switch's guarantee is an important fact to state > clearly in your series! > Yes, ports with lower index take priority. > > Hence, if the MAC of the DSA master drops some of these frames, it > > does not "spoil any chance" except if, out of the sequence LL n -> > > META n -> LL n+1 -> META n+1, it persistently drops only META n and LL > > n+1. > > LL = link layer? > Yes, link-local in this case means trapped frames in the 01-80-C2-xx-xx-xx or 01:1B:C9:xx:xx:xx space. > > So I'd like to re-state the problem towards what should be done to > > prevent LL and META frames getting reordered in the DSA master driver > > on multi-queue/multi-core systems. > > Ok. > > > At the most basic level, there > > should exist a rule that makes only a single core process these > > frames. > > This can be done simply using a data structure in the driver with an > appropriate locking mechanism. Then you don't have to worry which > core the driver code runs on. > Actually you do. DSA is special because it is not the first net device in the RX path that processes the frames. Something needs to be done on the master port. > Thanks, > Richard
Re: [PATCH net-next 0/5] PTP support for the SJA1105 DSA driver
On Fri, May 31, 2019 at 04:23:24PM +0300, Vladimir Oltean wrote: > The switch has internal logic to not send any other frame to the CPU > between a link-local and a meta frame. So this is guarantied by the switch? What happens when multiple PTP frames arrive at the same time on different ports? Does the switch buffer them and ensure strict ordering at the CPU port? In any case, the switch's guarantee is an important fact to state clearly in your series! > Hence, if the MAC of the DSA master drops some of these frames, it > does not "spoil any chance" except if, out of the sequence LL n -> > META n -> LL n+1 -> META n+1, it persistently drops only META n and LL > n+1. LL = link layer? > So I'd like to re-state the problem towards what should be done to > prevent LL and META frames getting reordered in the DSA master driver > on multi-queue/multi-core systems. Ok. > At the most basic level, there > should exist a rule that makes only a single core process these > frames. This can be done simply using a data structure in the driver with an appropriate locking mechanism. Then you don't have to worry which core the driver code runs on. Thanks, Richard
Re: [PATCH net-next 0/5] PTP support for the SJA1105 DSA driver
On Fri, 31 May 2019 at 07:34, Richard Cochran wrote: > > On Thu, May 30, 2019 at 06:23:09PM +0300, Vladimir Oltean wrote: > > On Thu, 30 May 2019 at 18:06, Richard Cochran > > wrote: > > > > > > But are the frames received in the same order? What happens your MAC > > > drops a frame? > > > > > > > If it drops a normal frame, it carries on. > > If it drops a meta frame, it prints "Expected meta frame", resets the > > state machine and carries on. > > If it drops a timestampable frame, it prints "Unexpected meta frame", > > resets the state machine and carries on. > > What I meant was, consider how dropped frames in the MAC will spoil > any chance that the driver has to correctly match time stamps with > frames. > > Thanks, > Richard I think you are still looking at this through the perspective of sequence numbers. I am *not* proposing to add sequence numbers for link-local and for meta in software, and then try to match them, as that would lead to complete chaos as you're suggesting. The switch has internal logic to not send any other frame to the CPU between a link-local and a meta frame. Hence, if the MAC of the DSA master drops some of these frames, it does not "spoil any chance" except if, out of the sequence LL n -> META n -> LL n+1 -> META n+1, it persistently drops only META n and LL n+1. If it drops less than 2 frames, the system recovers. And if it drops more, oh well, there are more pressing issues to deal with.. So I'd like to re-state the problem towards what should be done to prevent LL and META frames getting reordered in the DSA master driver on multi-queue/multi-core systems. At the most basic level, there should exist a rule that makes only a single core process these frames. Regards, -Vladimir
Re: [PATCH net-next 0/5] PTP support for the SJA1105 DSA driver
On Thu, May 30, 2019 at 06:23:09PM +0300, Vladimir Oltean wrote: > On Thu, 30 May 2019 at 18:06, Richard Cochran > wrote: > > > > But are the frames received in the same order? What happens your MAC > > drops a frame? > > > > If it drops a normal frame, it carries on. > If it drops a meta frame, it prints "Expected meta frame", resets the > state machine and carries on. > If it drops a timestampable frame, it prints "Unexpected meta frame", > resets the state machine and carries on. What I meant was, consider how dropped frames in the MAC will spoil any chance that the driver has to correctly match time stamps with frames. Thanks, Richard
Re: [PATCH net-next 0/5] PTP support for the SJA1105 DSA driver
On Thu, 30 May 2019 at 18:06, Richard Cochran wrote:
>
> On Thu, May 30, 2019 at 05:57:30PM +0300, Vladimir Oltean wrote:
> > On Thu, 30 May 2019 at 17:30, Richard Cochran
> > wrote:
> > >
> > > Not necessarily. If two frames that arrive at nearly the same time
> > > get their timestamps mixed up, that would be enough to break the time
> > > values but without breaking your state machine.
> > >
> >
> > This doesn't exactly sound like the type of thing I can check for.
>
> And that is why it cannot work.
>
> > The RX and TX timestamps *are* monotonically increasing with time for
> > all frames when I'm printing them in the {rx,tx}tstamp callbacks.
>
> But are the frames received in the same order? What happens your MAC
> drops a frame?
>
If it drops a normal frame, it carries on.
If it drops a meta frame, it prints "Expected meta frame", resets the
state machine and carries on.
If it drops a timestampable frame, it prints "Unexpected meta frame",
resets the state machine and carries on.
This doesn't happen under correct runtime conditions though.
-Vladimir
> > The driver returns free-running timestamps altered with a timecounter
> > frequency set by adjfine and offset set by adjtime.
>
> That should be correct.
>
> Thanks,
> Richard
Re: [PATCH net-next 0/5] PTP support for the SJA1105 DSA driver
On Thu, May 30, 2019 at 05:57:30PM +0300, Vladimir Oltean wrote:
> On Thu, 30 May 2019 at 17:30, Richard Cochran
> wrote:
> >
> > Not necessarily. If two frames that arrive at nearly the same time
> > get their timestamps mixed up, that would be enough to break the time
> > values but without breaking your state machine.
> >
>
> This doesn't exactly sound like the type of thing I can check for.
And that is why it cannot work.
> The RX and TX timestamps *are* monotonically increasing with time for
> all frames when I'm printing them in the {rx,tx}tstamp callbacks.
But are the frames received in the same order? What happens your MAC
drops a frame?
> The driver returns free-running timestamps altered with a timecounter
> frequency set by adjfine and offset set by adjtime.
That should be correct.
Thanks,
Richard
Re: [PATCH net-next 0/5] PTP support for the SJA1105 DSA driver
On Thu, 30 May 2019 at 17:30, Richard Cochran wrote:
>
> On Thu, May 30, 2019 at 12:01:23PM +0300, Vladimir Oltean wrote:
> > In fact that's why it doesn't work: because linuxptp adds ptp_dst_mac
> > (01-1B-19-00-00-00) and (01-80-C2-00-00-0E) to the MAC's multicast
> > filter, but the switch in its great wisdom mangles bytes
> > 01-1B-19-xx-xx-00 of the DMAC to place the switch id and source port
> > there (a rudimentary tagging mechanism). So the frames are no longer
> > accepted by this multicast MAC filter on the DSA master port unless
> > it's put in ALLMULTI or PROMISC.
>
> IOW, it is not linuxptp's choice to use these modes, but rather this
> is caused by a limitation of your device.
>
Didn't want to suggest otherwise. I'll see how I'm going to address that.
> > If the meta frames weren't associated with the correct link-local
> > frame, then the whole expect_meta -> SJA1105_STATE_META_ARRIVED
> > mechanism would go haywire, but it doesn't.
>
> Not necessarily. If two frames that arrive at nearly the same time
> get their timestamps mixed up, that would be enough to break the time
> values but without breaking your state machine.
>
This doesn't exactly sound like the type of thing I can check for.
The RX and TX timestamps *are* monotonically increasing with time for
all frames when I'm printing them in the {rx,tx}tstamp callbacks.
> > I was actually thinking it has something to do with the fact that I
> > shouldn't apply frequency corrections on timestamps of PTP delay
> > messages. Does that make any sense?
>
> What do you mean by that? Is the driver altering PTP message fields?
No.
The driver returns free-running timestamps altered with a timecounter
frequency set by adjfine and offset set by adjtime.
I was wondering out loud if there's any value in identifying delay
messages in order to not apply this frequency adjustment for their
timestamps.
-Vladimir
>
> Thanks,
> Richard
Re: [PATCH net-next 0/5] PTP support for the SJA1105 DSA driver
On Thu, May 30, 2019 at 12:01:23PM +0300, Vladimir Oltean wrote: > In fact that's why it doesn't work: because linuxptp adds ptp_dst_mac > (01-1B-19-00-00-00) and (01-80-C2-00-00-0E) to the MAC's multicast > filter, but the switch in its great wisdom mangles bytes > 01-1B-19-xx-xx-00 of the DMAC to place the switch id and source port > there (a rudimentary tagging mechanism). So the frames are no longer > accepted by this multicast MAC filter on the DSA master port unless > it's put in ALLMULTI or PROMISC. IOW, it is not linuxptp's choice to use these modes, but rather this is caused by a limitation of your device. > If the meta frames weren't associated with the correct link-local > frame, then the whole expect_meta -> SJA1105_STATE_META_ARRIVED > mechanism would go haywire, but it doesn't. Not necessarily. If two frames that arrive at nearly the same time get their timestamps mixed up, that would be enough to break the time values but without breaking your state machine. > I was actually thinking it has something to do with the fact that I > shouldn't apply frequency corrections on timestamps of PTP delay > messages. Does that make any sense? What do you mean by that? Is the driver altering PTP message fields? Thanks, Richard
Re: [PATCH net-next 0/5] PTP support for the SJA1105 DSA driver
On Thu, 30 May 2019 at 06:45, Richard Cochran wrote: > > On Wed, May 29, 2019 at 11:41:22PM +0300, Vladimir Oltean wrote: > > I'm sorry, then what does this code from raw.c do? > > It is a fallback for HW that doesn't support multicast filtering. > > Care to look a few lines above? If you did, you would have seen this: > > memset(, 0, sizeof(mreq)); > mreq.mr_ifindex = index; > mreq.mr_type = PACKET_MR_MULTICAST; > mreq.mr_alen = MAC_LEN; > memcpy(mreq.mr_address, addr1, MAC_LEN); > > err1 = setsockopt(fd, SOL_PACKET, option, , sizeof(mreq)); > You're right. In fact that's why it doesn't work: because linuxptp adds ptp_dst_mac (01-1B-19-00-00-00) and (01-80-C2-00-00-0E) to the MAC's multicast filter, but the switch in its great wisdom mangles bytes 01-1B-19-xx-xx-00 of the DMAC to place the switch id and source port there (a rudimentary tagging mechanism). So the frames are no longer accepted by this multicast MAC filter on the DSA master port unless it's put in ALLMULTI or PROMISC. > > > No. The root cause is the time stamps delivered by the hardware or > > > your driver. That needs to be addressed before going forward. > > > > > > > How can I check that the timestamps are valid? > > Well, you can see that there is something wrong. Perhaps you are not > matching the meta frames to the received packets. That is one > possible explanation, but you'll have to figure out what is happening. > If the meta frames weren't associated with the correct link-local frame, then the whole expect_meta -> SJA1105_STATE_META_ARRIVED mechanism would go haywire, but it doesn't. I was actually thinking it has something to do with the fact that I shouldn't apply frequency corrections on timestamps of PTP delay messages. Does that make any sense? Regards, -Vladimir > Thanks, > Richard
Re: [PATCH net-next 0/5] PTP support for the SJA1105 DSA driver
On Wed, May 29, 2019 at 11:41:22PM +0300, Vladimir Oltean wrote: > I'm sorry, then what does this code from raw.c do? It is a fallback for HW that doesn't support multicast filtering. Care to look a few lines above? If you did, you would have seen this: memset(, 0, sizeof(mreq)); mreq.mr_ifindex = index; mreq.mr_type = PACKET_MR_MULTICAST; mreq.mr_alen = MAC_LEN; memcpy(mreq.mr_address, addr1, MAC_LEN); err1 = setsockopt(fd, SOL_PACKET, option, , sizeof(mreq)); > > No. The root cause is the time stamps delivered by the hardware or > > your driver. That needs to be addressed before going forward. > > > > How can I check that the timestamps are valid? Well, you can see that there is something wrong. Perhaps you are not matching the meta frames to the received packets. That is one possible explanation, but you'll have to figure out what is happening. Thanks, Richard
Re: [PATCH net-next 0/5] PTP support for the SJA1105 DSA driver
On 5/29/19 7:52 AM, Richard Cochran wrote:
On Wed, May 29, 2019 at 02:56:22AM +0300, Vladimir Oltean wrote:
Not all is rosy, though.
You can sure say that again!
PTP timestamping will only work when the ports are bridged. Otherwise,
the metadata follow-up frames holding RX timestamps won't be received
because they will be blocked by the master port's MAC filter. Linuxptp
tries to put the net device in ALLMULTI/PROMISC mode,
Untrue.
I'm sorry, then what does this code from raw.c do?
mreq.mr_ifindex = index;
mreq.mr_type = PACKET_MR_ALLMULTI;
mreq.mr_alen = 0;
if (!setsockopt(fd, SOL_PACKET, option, , sizeof(mreq))) {
return 0;
}
pr_warning("setsockopt PACKET_MR_ALLMULTI failed: %m");
mreq.mr_ifindex = index;
mreq.mr_type = PACKET_MR_PROMISC;
mreq.mr_alen = 0;
if (!setsockopt(fd, SOL_PACKET, option, , sizeof(mreq))) {
return 0;
}
pr_warning("setsockopt PACKET_MR_PROMISC failed: %m");
but DSA doesn't
pass this on to the master port, which does the actual reception.
The master port is put in promiscous mode when the slave ports are
enslaved to a bridge.
Also, even with software-corrected timestamps, one can observe a
negative path delay reported by linuxptp:
ptp4l[55.600]: master offset 8 s2 freq +83677 path delay -2390
ptp4l[56.600]: master offset 17 s2 freq +83688 path delay -2391
ptp4l[57.601]: master offset 6 s2 freq +83682 path delay -2391
ptp4l[58.601]: master offset -1 s2 freq +83677 path delay -2391
Without investigating too deeply, this appears to be introduced by the
correction applied by linuxptp to t4 (t4c: corrected master rxtstamp)
during the path delay estimation process (removing the correction makes
the path delay positive).
No. The root cause is the time stamps delivered by the hardware or
your driver. That needs to be addressed before going forward.
How can I check that the timestamps are valid?
Regards,
-Vladimir
Thanks,
Richard
Re: [PATCH net-next 0/5] PTP support for the SJA1105 DSA driver
On Wed, May 29, 2019 at 02:56:22AM +0300, Vladimir Oltean wrote: > Not all is rosy, though. You can sure say that again! > PTP timestamping will only work when the ports are bridged. Otherwise, > the metadata follow-up frames holding RX timestamps won't be received > because they will be blocked by the master port's MAC filter. Linuxptp > tries to put the net device in ALLMULTI/PROMISC mode, Untrue. > but DSA doesn't > pass this on to the master port, which does the actual reception. > The master port is put in promiscous mode when the slave ports are > enslaved to a bridge. > > Also, even with software-corrected timestamps, one can observe a > negative path delay reported by linuxptp: > > ptp4l[55.600]: master offset 8 s2 freq +83677 path delay -2390 > ptp4l[56.600]: master offset 17 s2 freq +83688 path delay -2391 > ptp4l[57.601]: master offset 6 s2 freq +83682 path delay -2391 > ptp4l[58.601]: master offset -1 s2 freq +83677 path delay -2391 > > Without investigating too deeply, this appears to be introduced by the > correction applied by linuxptp to t4 (t4c: corrected master rxtstamp) > during the path delay estimation process (removing the correction makes > the path delay positive). No. The root cause is the time stamps delivered by the hardware or your driver. That needs to be addressed before going forward. Thanks, Richard
[PATCH net-next 0/5] PTP support for the SJA1105 DSA driver
This patchset adds the following: - A timecounter/cyclecounter based PHC for the free-running timestamping clock of this switch. - A state machine implemented in the DSA tagger for SJA1105, which keeps track of metadata follow-up Ethernet frames (the switch's way of transmitting RX timestamps). - Some common-sense on whether or not frames should be timestamped was taken out of the mv88e6xxx driver (the only other DSA driver with PTP support) and moved to the generic framework. An option was also added for drivers to override these common-sense decisions, and timestamp some more frames. This was the path of least resistance after implementing the aforementioned state machine - metadata follow-up frames need to be tracked anyway even if only to discard them and not pass them up the network stack. And since the switch can't just be told to timestamp only what the kernel wants (PTP frames), simply use all the timestamps it provides. - A generic helper in the timecounter/cyclecounter code for reconstructing partial PTP timestamps, such as those generated by the SJA1105. Not all is rosy, though. PTP timestamping will only work when the ports are bridged. Otherwise, the metadata follow-up frames holding RX timestamps won't be received because they will be blocked by the master port's MAC filter. Linuxptp tries to put the net device in ALLMULTI/PROMISC mode, but DSA doesn't pass this on to the master port, which does the actual reception. The master port is put in promiscous mode when the slave ports are enslaved to a bridge. Also, even with software-corrected timestamps, one can observe a negative path delay reported by linuxptp: ptp4l[55.600]: master offset 8 s2 freq +83677 path delay -2390 ptp4l[56.600]: master offset 17 s2 freq +83688 path delay -2391 ptp4l[57.601]: master offset 6 s2 freq +83682 path delay -2391 ptp4l[58.601]: master offset -1 s2 freq +83677 path delay -2391 Without investigating too deeply, this appears to be introduced by the correction applied by linuxptp to t4 (t4c: corrected master rxtstamp) during the path delay estimation process (removing the correction makes the path delay positive). This does not appear to have an obvious negative effect upon the synchronization. Lastly, clock manipulations on the actual hardware PTP clock will have to be implemented anyway, for the TTEthernet block and the time-based ingress policer. Vladimir Oltean (5): timecounter: Add helper for reconstructing partial timestamps net: dsa: sja1105: Add support for the PTP clock net: dsa: mv88e6xxx: Let taggers specify a can_timestamp function net: dsa: sja1105: Add support for PTP timestamping net: dsa: sja1105: Increase priority of CPU-trapped frames drivers/net/dsa/mv88e6xxx/hwtstamp.c | 25 +- drivers/net/dsa/mv88e6xxx/hwtstamp.h | 4 +- drivers/net/dsa/sja1105/Kconfig | 7 + drivers/net/dsa/sja1105/Makefile | 1 + drivers/net/dsa/sja1105/sja1105.h | 30 ++ .../net/dsa/sja1105/sja1105_dynamic_config.c | 2 + drivers/net/dsa/sja1105/sja1105_main.c| 272 - drivers/net/dsa/sja1105/sja1105_ptp.c | 357 ++ drivers/net/dsa/sja1105/sja1105_ptp.h | 48 +++ drivers/net/dsa/sja1105/sja1105_spi.c | 28 ++ .../net/dsa/sja1105/sja1105_static_config.c | 59 +++ .../net/dsa/sja1105/sja1105_static_config.h | 10 + include/linux/dsa/sja1105.h | 15 + include/linux/timecounter.h | 7 + include/net/dsa.h | 6 +- kernel/time/timecounter.c | 33 ++ net/dsa/dsa.c | 25 +- net/dsa/slave.c | 20 +- net/dsa/tag_sja1105.c | 135 ++- 19 files changed, 1043 insertions(+), 41 deletions(-) create mode 100644 drivers/net/dsa/sja1105/sja1105_ptp.c create mode 100644 drivers/net/dsa/sja1105/sja1105_ptp.h -- 2.17.1
