eclipse-threadx / netxduo

Eclipse ThreadX - NetXDuo is an advanced, industrial-grade TCP/IP network stack designed specifically for deeply embedded real-time and IoT applications
https://github.com/eclipse-threadx/rtos-docs/blob/main/rtos-docs/netx-duo/index.md
MIT License
242 stars 137 forks source link

PPP: missing PFC and ACFC support #136

Closed ohoc closed 1 year ago

ohoc commented 1 year ago

Hi,

While interfacing with a Telit GPRS module, I noticed that Protocol-Field-Compression (PFC) and Address-and-Control-Field-Compression (ACFC), as defined in RFC 1661, are not supported by nx_ppp.

I added support for both features and created PR #132 It would be nice if you could merge it :-)

Feel free to comment.

Best regards,

Olivier

bo-ms commented 1 year ago

Hi @ohoc thanks for your contribution, we will review the pull request and get back to you.

wenhui-xie commented 1 year ago

Hi @ohoc, did you encounter any issues with the current code? Why do you need the support of PFC or ACFC?

ohoc commented 1 year ago

Hi @wenhui-xie, Thanks for your reply. During LCP negotiations, the GPRS/4G module I use (Telit LE910S1) sends a LCP Configure-Request frame that requests ACFC and PFC. NetX PPP replies with a Configure-Ack frame. But NetX PPP is not able to decode subsequent frames that use ACFC and/or PFC. That is why I needed to add support for PFC and ACFC. Don't hesitate to ask if you want more details.

wenhui-xie commented 1 year ago

Hi @ohoc, thanks for your description. This may be the same issue as https://github.com/azure-rtos/netxduo/issues/100. According to RFC, these Configuration Options are sent to inform the peer that the implementation can receive such compressed fields. If our NetX PPP did not send this option to the peer, the peer should not send compressed data to NetX PPP.

ohoc commented 1 year ago

Hi @wenhui-xie Actually, the peer requests those configuration options, and NetX PPP accepts them in the configuration reply, instead of rejecting them. So there is something wrong here.

wenhui-xie commented 1 year ago

Hi @ohoc, thanks for your update. As RFC says if the peer sends these options means the peer can receive and process these options. NetX PPP accepts the options means it knows that the peer can process the options but doesn't mean NetX PPP can process the options. So if NetX PPP doesn't send these options to the peer, the peer should not send compressed data.

6.6. Address-and-Control-Field-Compression (ACFC)

  This Configuration Option provides a method to negotiate the
  compression of the Data Link Layer Address and Control fields.  By
  default, all implementations MUST transmit frames with Address and
  Control fields appropriate to the link framing.

  Since these fields usually have constant values for point-to-point
  links, they are easily compressed.  This Configuration Option is
  sent to inform the peer that the implementation can receive
  compressed Address and Control fields.

  If a compressed frame is received when Address-and-Control-Field-
  Compression has not been negotiated, the implementation MAY
  silently discard the frame.

  The Address and Control fields MUST NOT be compressed when sending
  any LCP packet.  This rule guarantees unambiguous recognition of
  LCP packets.

  When the Address and Control fields are compressed, the Data Link
  Layer FCS field is calculated on the compressed frame, not the
  original uncompressed frame.
bo-ms commented 1 year ago

Closing, feel free to open new issue if you have further issue.

wenhui-xie commented 1 year ago

Hi @ohoc , we have supported processing compressed PF and ACF. This is the latest update link of PPP: https://github.com/azure-rtos/netxduo/commit/e67c70ef358e36887bdb2ff7029807eaf234e97f. It would be much appreciated if you could help to test the latest code to check whether the compression issue you reported is fixed.

Below are the steps to test the new feature of PPP:

  1. Replace the nx_ppp.c and nx_ppp.h in your project with the latest files: https://github.com/azure-rtos/netxduo/tree/master/addons/ppp.
  2. Enable processing compressed PF and ACF by adding the macro 'NX_PPP_COMPRESSION_ENABLE' to nx_user.h or to the project configuration.
  3. Rebuild and run the project.