Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tunneling: L_Data.con Frames are not awaited properly before sending next L_Data.req #323

Closed
farmio opened this issue Aug 8, 2020 · 2 comments · Fixed by #822
Closed

Comments

@farmio
Copy link
Member

farmio commented Aug 8, 2020

I have noticed that, for tunnelling connections, every sent frame is processed twice. Once as OUTGOING and once as INCOMING frame.
When we send a TunnelingRequest the frame is sent as L_Data.req. Transmission shall be confirmed in L_Data.con. We don't handle incoming L_Data.req, L_Data.con and L_Data.ind differently so the confirmation (.con) is treated like an incoming telegram.

I'm not really sure what to do about this. As far as I understand this we should only process (as in fire callbacks) L_Data.ind frames. L_Data.con are just confirmations of our own frames and L_Data.req are only sent by the client (xknx), not by the tunneling server.

Shall we just ignore the L_Data.con frame and hope for the best? Handling this would probably require a second RequestResponse layer for tunneling...


From 3/3/3 Data Link Layer General §2.2:

There are 3 service primitives:

  1. L_Data.req shall be used to transmit a Frame.
  2. L_Data.ind shall be used to receive a Frame.
  3. L_Data.con shall be a local primitive generated by the local Layer-2 for its own client to indicate that the it is satisfied with the transmission.

If the local user of Layer-2 prepares an LSDU for the remote user it shall apply the L_Data.req primitive to pass the LSDU to the local Layer-2. The local Layer-2 shall accept the service request and shall try to send the LSDU to the remote Layer-2 with the relevant Frame format.
The local Layer-2 shall pass an L_Data.con primitive to the local user that shall indicate either a correct or erroneous data transfer. Depending if an L2-acknowledgement is requested or not, this confirmation shall be related to the reception of the L2-acknowledgement, or only to the transmission of the Frame on the medium.

From 3/6/3 EMI_IMI §4.1.5 Data Link Layer messages

4.1.5.1 Flow Control cEMI Client:

To keep the flow control for Data Link Layer services as simple as possible (this allows a simple flow control state machine in the cEMI client), it is recommended that:

  • a cEMI client sends a new Data Link Layer request only when the confirmation of the preceding request is received, or
  • a request-to-confirmation timeout is recognised; the recommended time-out for the cEMI client is 3 seconds.

A cEMI client shall at any time be able to accept an indication message from the cEMI Server.


Possibly the underlying issue of home-assistant/core#38130

@farmio
Copy link
Member Author

farmio commented Aug 10, 2020

It seems the L_Data.con should be awaited before sending next L_Data.req
(german) https://knx-user-forum.de/forum/öffentlicher-bereich/knx-eib-forum/1533948-l_data-con-beim-tunnel?p=1533985#post1533985

This would require some redesign of the TelegramQueue and KNXIPInterface classes.
The consumer of the underlying Queue in TelegramQueue is currently located in TelegramQueue.process_all_telegrams()
I think it should be located in the KNXIPInterface or better Tunnel class (Routing doesn't require this at all but there are RoutingBusy frames doing something alike that are not implemented currently). This would require proper separation of outgoing and incoming Telegram queue (see eg. #304 1893dbb ). It could maybe even replace the current rate_limiter (or we disable it per default then).


As a temporary workaround I suggest to check against the CEMIMessageCode somewhere here

telegram = knxipframe.body.cemi.telegram
and just return if it is L_Data_CON so the ACK is sent but the callbacks aren't processed again.

@farmio farmio changed the title Outgoing TunnelingRequest frames are processed twice Tunneling: L_Data.con Frames are not awaited properly before sending next L_Data.req Aug 18, 2020
@github-actions
Copy link
Contributor

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Please make sure to update to the latest version of xknx (or Home Assistant) and check if that solves the issue. Let us know if that works for you by adding a comment 👍 This issue has now been marked as stale and will be closed if no further activity occurs. Thank you for your contributions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant