[lwip-devel] [patch #8869] ppp: fix broken ppp_netif_output

[Stephan Linz]

Follow-up Comment #2, patch #8869 (project lwip):

Hi Sylvian

> +#if CCP_SUPPORT
> #if MPPE_SUPPORT
> 
> #endif /* MPPE_SUPPORT */
> +#endif /* CCP_SUPPORT */
> 
> That's not necessary, as per opt.h: 

Ok, I'll fix that. That was borrowed from ppp_input(). But you are right,
opt.h will cache this already.

> - if (pcb->ccp_is_up) {
> + if (pcb->ccp_is_up && pcb->ccp_transmit_method != 0) {
> 
> I don't think that's the proper fix, if CCP failed to establish,
> pcb->ccp_is_up MUST be false, if that's not the case, this is a bug,
> and this is this bug which must be fixed.

Where can I find the CCP establish and negotiation code. Maybe there is the
real bug.

> 
> Did you reach a case where pcb->ccp_is_up is true but
pcb->ccp_transmit_method
> isn't set to an available compressor (we only support MPPE so it should
only
> be 0 or MPPE) ?

Yes, see below (on Linux hosted PPP):

Hm, actually I use the unixsim example from lwip-contrib without any changes
in PPP daemon startup (see sio.c). This example should establish a PPP link
over a PTY without any compression. That mean CCP support was enabled, but the
PPP daemon doesn't support MPPE or MPPC.

After the LPC link up and the CCP negotiation, both values
pcb->ccp_transmit_method and pcb->ccp_receive_method will set to zero in
function ccp_set() (as expected). In ppp_input() that is not a problem,
because an "CCP is up" state is coupled in the right way with the PPP
protocol, only PPP_COMP (0xfd for MMPE) will be evaluate in the case
statement:

  /* IN MY CASE LINUX PPPD WILL NEVER SEND A PPP_COMP PACKAGE */
  if (protocol == PPP_COMP) {
    /* ... */
    switch (pcb->ccp_receive_method) {
      /* ... */
    default:
      goto drop;
    }
  }

As long as the PPP damon on the Linux host side will never send MPPE
encoded/compressed packages that will work. But in ppp_netif_output() it is
completely different (why I run into this unexpected case you ask above):

  /* IN CASE OF LWIP PPP WE WILL NEVER SEE PPP_COMP PACKAGE HERE (BUG?) */
  if (pcb->ccp_is_up) {
    /* ... */
    switch (pcb->ccp_transmit_method) {
      /* ... */
    default:
      goto drop;
    }
  }

Apart of VJ compression there is no further protocol type definition. To me,
that would be a further bug, because in the case of supported and required
MMPE the protocol type should be changed to PPP_COMP, or not?

But my problem is: CCP is open and up but no supported compression method was
negotiated. To me that looks like a common case. A PPP daemon can try to use
compression (so it use CCP) but the lwip side will not support all
compression/encoding methods. Then the switch statement in ppp_netif_output()
will drop all PPP_IP type packages.


Best Regards,
Stepahn

    _______________________________________________________

Reply to this item at:

  <http://savannah.nongnu.org/patch/?8869>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.nongnu.org/